From c087ee35f700d36811681a4eea57f7da7cd38484 Mon Sep 17 00:00:00 2001
From: Bjoern Rabenstein <bjoern@soundcloud.com>
Date: Mon, 10 Nov 2014 18:33:31 +0100
Subject: [PATCH] Remove archiveMtx.

Change-Id: Ie8019f860bbda68621f74380c90a4e57930d3d7a
---
 storage/local/persistence.go | 37 ++++++++++++++----------------------
 storage/local/storage.go     | 11 -----------
 2 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/storage/local/persistence.go b/storage/local/persistence.go
index 646075dbc..328f4b6a8 100644
--- a/storage/local/persistence.go
+++ b/storage/local/persistence.go
@@ -86,11 +86,6 @@ type persistence struct {
 	basePath string
 	chunkLen int
 
-	// archiveMtx protects the archiving-related methods archiveMetric,
-	// unarchiveMetric, dropArchiveMetric, and getFingerprintsModifiedBefore
-	// from concurrent calls.
-	archiveMtx sync.Mutex
-
 	archivedFingerprintToMetrics   *index.FingerprintMetricIndex
 	archivedFingerprintToTimeRange *index.FingerprintTimeRangeIndex
 	labelPairToFingerprints        *index.LabelPairFingerprintIndex
@@ -1155,13 +1150,10 @@ func (p *persistence) waitForIndexing() {
 
 // archiveMetric persists the mapping of the given fingerprint to the given
 // metric, together with the first and last timestamp of the series belonging to
-// the metric. This method is goroutine-safe.
+// the metric. The caller must have locked the fingerprint.
 func (p *persistence) archiveMetric(
 	fp clientmodel.Fingerprint, m clientmodel.Metric, first, last clientmodel.Timestamp,
 ) error {
-	p.archiveMtx.Lock()
-	defer p.archiveMtx.Unlock()
-
 	if err := p.archivedFingerprintToMetrics.Put(codable.Fingerprint(fp), codable.Metric(m)); err != nil {
 		p.setDirty(true)
 		return err
@@ -1196,12 +1188,6 @@ func (p *persistence) updateArchivedTimeRange(
 // that have live samples before the provided timestamp. This method is
 // goroutine-safe.
 func (p *persistence) getFingerprintsModifiedBefore(beforeTime clientmodel.Timestamp) ([]clientmodel.Fingerprint, error) {
-	// The locking makes sure archivedFingerprintToTimeRange won't be
-	// mutated while being iterated over (which will probably not result in
-	// races, but might still yield weird results).
-	p.archiveMtx.Lock()
-	defer p.archiveMtx.Unlock()
-
 	var fp codable.Fingerprint
 	var tr codable.TimeRange
 	fps := []clientmodel.Fingerprint{}
@@ -1229,7 +1215,8 @@ func (p *persistence) getArchivedMetric(fp clientmodel.Fingerprint) (clientmodel
 
 // dropArchivedMetric deletes an archived fingerprint and its corresponding
 // metric entirely. It also queues the metric for un-indexing (no need to call
-// unindexMetric for the deleted metric.)  This method is goroutine-safe.
+// unindexMetric for the deleted metric.)  The caller must have locked the
+// fingerprint.
 func (p *persistence) dropArchivedMetric(fp clientmodel.Fingerprint) (err error) {
 	defer func() {
 		if err != nil {
@@ -1237,9 +1224,6 @@ func (p *persistence) dropArchivedMetric(fp clientmodel.Fingerprint) (err error)
 		}
 	}()
 
-	p.archiveMtx.Lock()
-	defer p.archiveMtx.Unlock()
-
 	metric, err := p.getArchivedMetric(fp)
 	if err != nil || metric == nil {
 		return err
@@ -1257,10 +1241,17 @@ func (p *persistence) dropArchivedMetric(fp clientmodel.Fingerprint) (err error)
 // unarchiveMetric deletes an archived fingerprint and its metric, but (in
 // contrast to dropArchivedMetric) does not un-index the metric.  If a metric
 // was actually deleted, the method returns true and the first time of the
-// deleted metric. This method is goroutine-safe.
-func (p *persistence) unarchiveMetric(fp clientmodel.Fingerprint) (bool, clientmodel.Timestamp, error) {
-	p.archiveMtx.Lock()
-	defer p.archiveMtx.Unlock()
+// deleted metric. The caller must have locked the fingerprint.
+func (p *persistence) unarchiveMetric(fp clientmodel.Fingerprint) (
+	deletedAnything bool,
+	firstDeletedTime clientmodel.Timestamp,
+	err error,
+) {
+	defer func() {
+		if err != nil {
+			p.setDirty(true)
+		}
+	}()
 
 	firstTime, _, has, err := p.archivedFingerprintToTimeRange.Lookup(fp)
 	if err != nil || !has {
diff --git a/storage/local/storage.go b/storage/local/storage.go
index a3db42841..d1e6302bc 100644
--- a/storage/local/storage.go
+++ b/storage/local/storage.go
@@ -359,7 +359,6 @@ func (s *memorySeriesStorage) getOrCreateSeries(fp clientmodel.Fingerprint, m cl
 		unarchived, firstTime, err := s.persistence.unarchiveMetric(fp)
 		if err != nil {
 			glog.Errorf("Error unarchiving fingerprint %v: %v", fp, err)
-			s.persistence.setDirty(true)
 		}
 		if unarchived {
 			s.seriesOps.WithLabelValues(unarchive).Inc()
@@ -375,16 +374,6 @@ func (s *memorySeriesStorage) getOrCreateSeries(fp clientmodel.Fingerprint, m cl
 	return series
 }
 
-/*
-func (s *memorySeriesStorage) preloadChunksAtTime(fp clientmodel.Fingerprint, ts clientmodel.Timestamp) (chunkDescs, error) {
-	series, ok := s.fpToSeries.get(fp)
-	if !ok {
-		panic("requested preload for non-existent series")
-	}
-	return series.preloadChunksAtTime(ts, s.persistence)
-}
-*/
-
 func (s *memorySeriesStorage) preloadChunksForRange(
 	fp clientmodel.Fingerprint,
 	from clientmodel.Timestamp, through clientmodel.Timestamp,