From 050f0147823a28cdfaa2b95e85f65634ef3666e4 Mon Sep 17 00:00:00 2001 From: SuperQ Date: Wed, 23 Oct 2024 15:29:06 +0200 Subject: [PATCH] Improve replica flag handling Add a string utility parsing function to improve the handling of replica label flags. This allows for easier handling of flags when multiple replica labels are need. * Split flag parts that are comma separated. * Remove any empty strings. * Sort and deduplicate the slice. For example in the case of multiple replica labels like: `--query.replica-label=prometheus_replica,thanos_rule_replica` Signed-off-by: SuperQ --- CHANGELOG.md | 1 + cmd/thanos/compact.go | 15 +++++++------ cmd/thanos/query.go | 6 +++++- docs/components/compact.md | 18 +++++++++------- docs/components/query.md | 2 ++ pkg/strutil/labels.go | 25 ++++++++++++++++++++++ pkg/strutil/labels_test.go | 43 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 pkg/strutil/labels.go create mode 100644 pkg/strutil/labels_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b548e4d9b..c8e9d91a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#7652](https://github.com/thanos-io/thanos/pull/7652) Store: Implement metadata API limit in stores. - [#7659](https://github.com/thanos-io/thanos/pull/7659) Receive: Add support for replication using [Cap'n Proto](https://capnproto.org/). This protocol has a lower CPU and memory footprint, which leads to a reduction in resource usage in Receivers. Before enabling it, make sure that all receivers are updated to a version which supports this replication method. - [#7853](https://github.com/thanos-io/thanos/pull/7853) UI: Add support for selecting graph time range with mouse drag. +- [#7855](https://github.com/thanos-io/thanos/pull/7855) Compcat/Query: Add support for comma separated replica labels. ### Changed diff --git a/cmd/thanos/compact.go b/cmd/thanos/compact.go index 06d3ffaaa3..ce568b4314 100644 --- a/cmd/thanos/compact.go +++ b/cmd/thanos/compact.go @@ -46,6 +46,7 @@ import ( "github.com/thanos-io/thanos/pkg/runutil" httpserver "github.com/thanos-io/thanos/pkg/server/http" "github.com/thanos-io/thanos/pkg/store" + "github.com/thanos-io/thanos/pkg/strutil" "github.com/thanos-io/thanos/pkg/tracing" "github.com/thanos-io/thanos/pkg/ui" ) @@ -254,10 +255,11 @@ func runCompact( } enableVerticalCompaction := conf.enableVerticalCompaction - if len(conf.dedupReplicaLabels) > 0 { + dedupReplicaLabels := strutil.ParseFlagLabels(conf.dedupReplicaLabels) + if len(dedupReplicaLabels) > 0 { enableVerticalCompaction = true level.Info(logger).Log( - "msg", "deduplication.replica-label specified, enabling vertical compaction", "dedupReplicaLabels", strings.Join(conf.dedupReplicaLabels, ","), + "msg", "deduplication.replica-label specified, enabling vertical compaction", "dedupReplicaLabels", strings.Join(dedupReplicaLabels, ","), ) } if enableVerticalCompaction { @@ -275,7 +277,7 @@ func runCompact( labelShardedMetaFilter, consistencyDelayMetaFilter, ignoreDeletionMarkFilter, - block.NewReplicaLabelRemover(logger, conf.dedupReplicaLabels), + block.NewReplicaLabelRemover(logger, dedupReplicaLabels), duplicateBlocksFilter, noCompactMarkerFilter, } @@ -326,7 +328,7 @@ func runCompact( case compact.DedupAlgorithmPenalty: mergeFunc = dedup.NewChunkSeriesMerger() - if len(conf.dedupReplicaLabels) == 0 { + if len(dedupReplicaLabels) == 0 { return errors.New("penalty based deduplication needs at least one replica label specified") } case "": @@ -818,8 +820,9 @@ 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, "") - 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 one or more labels are set, compactor will ignore the given labels so that vertical compaction can merge the blocks."+ + cmd.Flag("deduplication.replica-label", "Experimental. 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. "+ + "Flag may be specified multiple times as well as a comma separated list of labels. "+ + "When one or more labels are set, compactor will ignore the given labels so that vertical compaction can merge the blocks."+ "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 different deduplication algorithm (e.g one that works well with Prometheus replicas), please set it via --deduplication.func."). StringsVar(&cc.dedupReplicaLabels) diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index c80db1cd23..d9731ab489 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -56,6 +56,7 @@ import ( httpserver "github.com/thanos-io/thanos/pkg/server/http" "github.com/thanos-io/thanos/pkg/store" "github.com/thanos-io/thanos/pkg/store/labelpb" + "github.com/thanos-io/thanos/pkg/strutil" "github.com/thanos-io/thanos/pkg/targets" "github.com/thanos-io/thanos/pkg/tenancy" "github.com/thanos-io/thanos/pkg/tls" @@ -122,7 +123,7 @@ func registerQuery(app *extkingpin.App) { Default(string(query.ExternalLabels), string(query.StoreType)). Enums(string(query.ExternalLabels), string(query.StoreType)) - queryReplicaLabels := cmd.Flag("query.replica-label", "Labels to treat as a replica indicator along which data is deduplicated. Still you will be able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules."). + queryReplicaLabels := cmd.Flag("query.replica-label", "Labels to treat as a replica indicator along which data is deduplicated. Still you will be able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules. Flag may be specified multiple times as well as a comma separated list of labels."). Strings() queryPartitionLabels := cmd.Flag("query.partition-label", "Labels that partition the leaf queriers. This is used to scope down the labelsets of leaf queriers when using the distributed query mode. If set, these labels must form a partition of the leaf queriers. Partition labels must not intersect with replica labels. Every TSDB of a leaf querier must have these labels. This is useful when there are multiple external labels that are irrelevant for the partition as it allows the distributed engine to ignore them for some optimizations. If this is empty then all labels are used as partition labels.").Strings() @@ -539,6 +540,9 @@ func runQuery( store.WithProxyStoreDebugLogging(debugLogging), } + // Parse and sanitize the provided replica labels flags. + queryReplicaLabels = strutil.ParseFlagLabels(queryReplicaLabels) + var ( endpoints = prepareEndpointSet( g, diff --git a/docs/components/compact.md b/docs/components/compact.md index aa47143a25..a53e2c85b4 100644 --- a/docs/components/compact.md +++ b/docs/components/compact.md @@ -342,14 +342,16 @@ Flags: At least one replica label has to be set via --deduplication.replica-label flag. --deduplication.replica-label=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 one or more labels are set, compactor - will ignore the given labels so that vertical - compaction can merge the blocks.Please note - that by default this uses a NAIVE algorithm + Experimental. 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. Flag may be specified multiple + times as well as a comma separated list of + labels. When one or more labels are set, + compactor will ignore the given labels so that + vertical compaction can merge the blocks.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 diff --git a/docs/components/query.md b/docs/components/query.md index 3b300b5d7a..31410d07ea 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -444,6 +444,8 @@ Flags: be able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules. + Flag may be specified multiple times as well as + a comma separated list of labels. --query.telemetry.request-duration-seconds-quantiles=0.1... ... The quantiles for exporting metrics about the request duration quantiles. diff --git a/pkg/strutil/labels.go b/pkg/strutil/labels.go new file mode 100644 index 0000000000..5423fd38eb --- /dev/null +++ b/pkg/strutil/labels.go @@ -0,0 +1,25 @@ +// Copyright (c) The Thanos Authors. +// Licensed under the Apache License 2.0. + +package strutil + +import ( + "slices" + "strings" +) + +// ParseFlagLabels helps handle lists of labels passed from kingpin flags. +// * Split flag parts that are comma separated. +// * Remove any empty strings. +// * Sort and deduplicate the slice. +func ParseFlagLabels(f []string) []string { + var result []string + for _, l := range f { + if l == "" { + continue + } + result = append(result, strings.Split(l, ",")...) + } + slices.Sort(result) + return slices.Compact(result) +} diff --git a/pkg/strutil/labels_test.go b/pkg/strutil/labels_test.go new file mode 100644 index 0000000000..159fb6763e --- /dev/null +++ b/pkg/strutil/labels_test.go @@ -0,0 +1,43 @@ +// Copyright (c) The Thanos Authors. +// Licensed under the Apache License 2.0. + +package strutil + +import ( + "testing" + + "github.com/efficientgo/core/testutil" +) + +func TestParseFlagLabels(t *testing.T) { + testCases := map[string]struct { + flags []string + expected []string + }{ + "single flag with commas": { + flags: []string{ + "a,b,c", + }, + expected: []string{"a", "b", "c"}, + }, + "multiple flags with commas": { + flags: []string{ + "a", "b", "c,d", + }, + expected: []string{"a", "b", "c", "d"}, + }, + "multiple flags empty strings": { + flags: []string{ + "a", "b", "", + }, + expected: []string{"a", "b"}, + }, + } + + for tcName, tc := range testCases { + t.Run(tcName, func(t *testing.T) { + res := ParseFlagLabels(tc.flags) + testutil.Equals(t, tc.expected, res) + }) + } +}