address review comment

Signed-off-by: yeya24 <[email protected]>
thanos-io · May 21, 2021 · 1a2f36a · 1a2f36a
1 parent 666c8c0
commit 1a2f36a
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 10 deletions.
diff --git a/cmd/thanos/compact.go b/cmd/thanos/compact.go
@@ -309,7 +309,7 @@ func runCompact(
 	var mergeFunc storage.VerticalChunkSeriesMergeFunc
 	switch conf.dedupFunc {
 	case compact.DedupAlgorithmPenalty:
-		mergeFunc = dedup.NewDedupChunkSeriesMerger()
+		mergeFunc = dedup.NewChunkSeriesMerger()
 
 		if len(conf.dedupReplicaLabels) == 0 {
 			return errors.New("penalty based deduplication needs at least one replica label specified")
@@ -638,7 +638,7 @@ func (cc *compactConfig) registerFlag(cmd extkingpin.FlagClause) {
 		Default("48h").SetValue(&cc.deleteDelay)
 
 	cmd.Flag("compact.enable-vertical-compaction", "Experimental. When set to true, compactor will allow overlaps and perform **irreversible** vertical compaction. See https://thanos.io/tip/components/compact.md/#vertical-compactions to read more."+
-		"Please note that this uses a NAIVE algorithm for merging. If you need a smarter deduplication algorithm, please set it via -- compact.dedup-func."+
+		"Please note that by default this uses a NAIVE algorithm for merging. If you need a smarter deduplication algorithm, please set it via -- compact.dedup-func."+
 		"NOTE: This flag is ignored and (enabled) when --deduplication.replica-label flag is set.").
 		Hidden().Default("false").BoolVar(&cc.enableVerticalCompaction)
 
@@ -647,11 +647,10 @@ func (cc *compactConfig) registerFlag(cmd extkingpin.FlagClause) {
 		"When set to penalty, penalty based deduplication algorithm will be used. At least one replica label has to be set via --deduplication.replica-label flag.").
 		Default("").EnumVar(&cc.dedupFunc, compact.DedupAlgorithmPenalty, "")
 
-	// Update this. This flag works for both dedup version compactor and the original compactor.
 	cmd.Flag("deduplication.replica-label", "Label to treat as a replica indicator of blocks that can be deduplicated (repeated flag). This will merge multiple replica blocks into one. This process is irreversible."+
 		"Experimental. When it is set to true, compactor will ignore the given labels so that vertical compaction can merge the blocks."+
-		"Please note that this uses a NAIVE algorithm for merging (no smart replica deduplication, just chaining samples together)."+
-		"This works well for deduplication of blocks with **precisely the same samples** like produced by Receiver replication.").
+		"Please note that by default this uses a NAIVE algorithm for merging which works well for deduplication of blocks with **precisely the same samples** like produced by Receiver replication."+
+		"If you need a smarter deduplication algorithm, please set it via -- compact.dedup-func.").
 		Hidden().StringsVar(&cc.dedupReplicaLabels)
 
 	// TODO(bwplotka): This is short term fix for https://github.com/thanos-io/thanos/issues/1424, replace with vertical block sharding https://github.com/thanos-io/thanos/pull/3390.

diff --git a/pkg/compact/compact_e2e_test.go b/pkg/compact/compact_e2e_test.go
@@ -176,7 +176,7 @@ func TestGroupCompactE2E(t *testing.T) {
 
 // Penalty based merger should get the same result as the blocks don't have overlap.
 func TestGroupCompactPenaltyDedupE2E(t *testing.T) {
-	testGroupCompactE2e(t, dedup.NewDedupChunkSeriesMerger())
+	testGroupCompactE2e(t, dedup.NewChunkSeriesMerger())
 }
 
 func testGroupCompactE2e(t *testing.T, mergeFunc storage.VerticalChunkSeriesMergeFunc) {

diff --git a/pkg/dedup/chunk_iter.go b/pkg/dedup/chunk_iter.go
@@ -14,9 +14,9 @@ import (
 	"github.com/thanos-io/thanos/pkg/compact/downsample"
 )
 
-// NewDedupChunkSeriesMerger merges several chunk series into one.
+// NewChunkSeriesMerger merges several chunk series into one.
 // Deduplication is based on penalty based deduplication algorithm without handling counter reset.
-func NewDedupChunkSeriesMerger() storage.VerticalChunkSeriesMergeFunc {
+func NewChunkSeriesMerger() storage.VerticalChunkSeriesMergeFunc {
 	return func(series ...storage.ChunkSeries) storage.ChunkSeries {
 		if len(series) == 0 {
 			return nil

diff --git a/pkg/dedup/chunk_iter_test.go b/pkg/dedup/chunk_iter_test.go
@@ -16,7 +16,7 @@ import (
 )
 
 func TestDedupChunkSeriesMerger(t *testing.T) {
-	m := NewDedupChunkSeriesMerger()
+	m := NewChunkSeriesMerger()
 
 	for _, tc := range []struct {
 		name     string
@@ -144,7 +144,7 @@ func TestDedupChunkSeriesMerger(t *testing.T) {
 }
 
 func TestDedupChunkSeriesMergerDownsampledChunks(t *testing.T) {
-	m := NewDedupChunkSeriesMerger()
+	m := NewChunkSeriesMerger()
 
 	defaultLabels := labels.FromStrings("bar", "baz")
 	emptySamples := downsample.SamplesFromTSDBSamples([]tsdbutil.Sample{})