Addressed comments.

Signed-off-by: Bartek Plotka <[email protected]>
thanos-io · Oct 18, 2019 · 1f47c6d · 1f47c6d
1 parent bfa698a
commit 1f47c6d
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 19 deletions.
diff --git a/pkg/compact/compact.go b/pkg/compact/compact.go
@@ -64,7 +64,8 @@ type syncerMetrics struct {
 	garbageCollectionFailures prometheus.Counter
 	garbageCollectionDuration prometheus.Histogram
 	compactions               *prometheus.CounterVec
-	compactionsRuns           *prometheus.CounterVec
+	compactionRunsStarted     *prometheus.CounterVec
+	compactionRunsFinished    *prometheus.CounterVec
 	compactionFailures        *prometheus.CounterVec
 }
 
@@ -111,9 +112,13 @@ func newSyncerMetrics(reg prometheus.Registerer) *syncerMetrics {
 		Name: "thanos_compact_group_compactions_total",
 		Help: "Total number of group compactions attempts, resulted with new block.",
 	}, []string{"group"})
-	m.compactionsRuns = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Name: "thanos_compact_group_compaction_runs_total",
-		Help: "Total number of group compactions run attempts. This also includes compactor group runs that resulted with no compaction.",
+	m.compactionRunsStarted = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Name: "thanos_compact_group_compaction_runs_started_total",
+		Help: "Total number of group compactions run attempts.",
+	}, []string{"group"})
+	m.compactionRunsFinished = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Name: "thanos_compact_group_compaction_runs_finished_total",
+		Help: "Total number of group compactions run finished. This also includes compactor group runs that resulted with no compaction.",
 	}, []string{"group"})
 	m.compactionFailures = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Name: "thanos_compact_group_compactions_failures_total",
@@ -130,7 +135,8 @@ func newSyncerMetrics(reg prometheus.Registerer) *syncerMetrics {
 			m.garbageCollectionFailures,
 			m.garbageCollectionDuration,
 			m.compactions,
-			m.compactionsRuns,
+			m.compactionRunsStarted,
+			m.compactionRunsFinished,
 			m.compactionFailures,
 		)
 	}
@@ -366,7 +372,8 @@ func (c *Syncer) Groups() (res []*Group, err error) {
 				m.Thanos.Downsample.Resolution,
 				c.acceptMalformedIndex,
 				c.metrics.compactions.WithLabelValues(GroupKey(m.Thanos)),
-				c.metrics.compactionsRuns.WithLabelValues(GroupKey(m.Thanos)),
+				c.metrics.compactionRunsStarted.WithLabelValues(GroupKey(m.Thanos)),
+				c.metrics.compactionRunsFinished.WithLabelValues(GroupKey(m.Thanos)),
 				c.metrics.compactionFailures.WithLabelValues(GroupKey(m.Thanos)),
 				c.metrics.garbageCollectedBlocks,
 			)
@@ -513,7 +520,8 @@ type Group struct {
 	blocks                      map[ulid.ULID]*metadata.Meta
 	acceptMalformedIndex        bool
 	compactions                 prometheus.Counter
-	compactionRuns              prometheus.Counter
+	compactionRunsStarted       prometheus.Counter
+	compactionRunsCompleted     prometheus.Counter
 	compactionFailures          prometheus.Counter
 	groupGarbageCollectedBlocks prometheus.Counter
 }
@@ -526,7 +534,8 @@ func newGroup(
 	resolution int64,
 	acceptMalformedIndex bool,
 	compactions prometheus.Counter,
-	compactionRuns prometheus.Counter,
+	compactionRunsStarted prometheus.Counter,
+	compactionRunsFinished prometheus.Counter,
 	compactionFailures prometheus.Counter,
 	groupGarbageCollectedBlocks prometheus.Counter,
 ) (*Group, error) {
@@ -541,7 +550,8 @@ func newGroup(
 		blocks:                      map[ulid.ULID]*metadata.Meta{},
 		acceptMalformedIndex:        acceptMalformedIndex,
 		compactions:                 compactions,
-		compactionRuns:              compactionRuns,
+		compactionRunsStarted:       compactionRunsStarted,
+		compactionRunsCompleted:     compactionRunsFinished,
 		compactionFailures:          compactionFailures,
 		groupGarbageCollectedBlocks: groupGarbageCollectedBlocks,
 	}
@@ -595,7 +605,7 @@ func (cg *Group) Resolution() int64 {
 // Compact plans and runs a single compaction against the group. The compacted result
 // is uploaded into the bucket the blocks were retrieved from.
 func (cg *Group) Compact(ctx context.Context, dir string, comp tsdb.Compactor) (bool, ulid.ULID, error) {
-	cg.compactionRuns.Inc()
+	cg.compactionRunsStarted.Inc()
 
 	subDir := filepath.Join(dir, cg.Key())
 
@@ -615,8 +625,10 @@ func (cg *Group) Compact(ctx context.Context, dir string, comp tsdb.Compactor) (
 	shouldRerun, compID, err := cg.compact(ctx, subDir, comp)
 	if err != nil {
 		cg.compactionFailures.Inc()
+		return false, ulid.ULID{}, err
 	}
-	return shouldRerun, compID, err
+	cg.compactionRunsCompleted.Inc()
+	return shouldRerun, compID, nil
 }
 
 // Issue347Error is a type wrapper for errors that should invoke repair process for broken block.

diff --git a/pkg/compact/compact_e2e_test.go b/pkg/compact/compact_e2e_test.go
@@ -166,9 +166,9 @@ func TestSyncer_GarbageCollect_e2e(t *testing.T) {
 		groups, err := sy.Groups()
 		testutil.Ok(t, err)
 
-		testutil.Equals(t, "0@{}", groups[0].Key())
+		testutil.Equals(t, "0@17241709254077376921", groups[0].Key())
 		testutil.Equals(t, []ulid.ULID{metas[9].ULID, m3.ULID}, groups[0].IDs())
-		testutil.Equals(t, "1000@{}", groups[1].Key())
+		testutil.Equals(t, "1000@17241709254077376921", groups[1].Key())
 		testutil.Equals(t, []ulid.ULID{m4.ULID}, groups[1].IDs())
 	})
 }
@@ -224,7 +224,8 @@ func TestGroup_Compact_e2e(t *testing.T) {
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.garbageCollectedBlocks))
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.garbageCollectionFailures))
 		testutil.Equals(t, 0, MetricCount(sy.metrics.compactions))
-		testutil.Equals(t, 0, MetricCount(sy.metrics.compactionsRuns))
+		testutil.Equals(t, 0, MetricCount(sy.metrics.compactionRunsStarted))
+		testutil.Equals(t, 0, MetricCount(sy.metrics.compactionRunsFinished))
 		testutil.Equals(t, 0, MetricCount(sy.metrics.compactionFailures))
 
 		_, err = os.Stat(dir)
@@ -317,12 +318,18 @@ func TestGroup_Compact_e2e(t *testing.T) {
 		testutil.Equals(t, 1.0, promtest.ToFloat64(sy.metrics.compactions.WithLabelValues(GroupKey(metas[7].Thanos))))
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.compactions.WithLabelValues(GroupKey(metas[4].Thanos))))
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.compactions.WithLabelValues(GroupKey(metas[5].Thanos))))
-		testutil.Equals(t, 4, MetricCount(sy.metrics.compactionsRuns))
-		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionsRuns.WithLabelValues(GroupKey(metas[0].Thanos))))
-		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionsRuns.WithLabelValues(GroupKey(metas[7].Thanos))))
+		testutil.Equals(t, 4, MetricCount(sy.metrics.compactionRunsStarted))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsStarted.WithLabelValues(GroupKey(metas[0].Thanos))))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsStarted.WithLabelValues(GroupKey(metas[7].Thanos))))
 		// TODO(bwplotka): Looks like we do some unnecessary loops. Not a major problem but investigate.
-		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionsRuns.WithLabelValues(GroupKey(metas[4].Thanos))))
-		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionsRuns.WithLabelValues(GroupKey(metas[5].Thanos))))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsStarted.WithLabelValues(GroupKey(metas[4].Thanos))))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsStarted.WithLabelValues(GroupKey(metas[5].Thanos))))
+		testutil.Equals(t, 4, MetricCount(sy.metrics.compactionRunsFinished))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsFinished.WithLabelValues(GroupKey(metas[0].Thanos))))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsFinished.WithLabelValues(GroupKey(metas[7].Thanos))))
+		// TODO(bwplotka): Looks like we do some unnecessary loops. Not a major problem but investigate.
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsFinished.WithLabelValues(GroupKey(metas[4].Thanos))))
+		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.compactionRunsFinished.WithLabelValues(GroupKey(metas[5].Thanos))))
 		testutil.Equals(t, 4, MetricCount(sy.metrics.compactionFailures))
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.compactionFailures.WithLabelValues(GroupKey(metas[0].Thanos))))
 		testutil.Equals(t, 0.0, promtest.ToFloat64(sy.metrics.compactionFailures.WithLabelValues(GroupKey(metas[7].Thanos))))