Skip to content

Commit

Permalink
fix ReplicaLabelRemover panic when replicaLabels are not specified (#…
Browse files Browse the repository at this point in the history
…2936)

Signed-off-by: Ben Ye <yb532204897@gmail.com>
  • Loading branch information
yeya24 authored Jul 27, 2020
1 parent a64884b commit 02672f3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel
- [#2866](https://github.com/thanos-io/thanos/pull/2866) Receive, Querier: Fixed leaks on receive and querier Store API Series, which were leaking on errors.
- [#2895](https://github.com/thanos-io/thanos/pull/2895) Compact: Fix increment of `thanos_compact_downsample_total` metric for downsample of 5m resolution blocks.
- [#2858](https://github.com/thanos-io/thanos/pull/2858) Store: Fix `--store.grpc.series-sample-limit` implementation. The limit is now applied to the sum of all samples fetched across all queried blocks via a single Series call, instead of applying it individually to each block.
- [#2936](https://github.com/thanos-io/thanos/pull/2936) Compact: Fix ReplicaLabelRemover panic when replicaLabels are not specified.

### Added

Expand Down
4 changes: 4 additions & 0 deletions pkg/block/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,10 @@ func NewReplicaLabelRemover(logger log.Logger, replicaLabels []string) *ReplicaL

// Modify modifies external labels of existing blocks, it removes given replica labels from the metadata of blocks that have it.
func (r *ReplicaLabelRemover) Modify(_ context.Context, metas map[ulid.ULID]*metadata.Meta, modified *extprom.TxGaugeVec) error {
if len(r.replicaLabels) == 0 {
return nil
}

for u, meta := range metas {
l := meta.Thanos.Labels
for _, replicaLabel := range r.replicaLabels {
Expand Down
33 changes: 25 additions & 8 deletions pkg/block/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,13 +878,13 @@ func TestDeduplicateFilter_Filter(t *testing.T) {
func TestReplicaLabelRemover_Modify(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
defer cancel()
rm := NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"})

for _, tcase := range []struct {
name string
input map[ulid.ULID]*metadata.Meta
expected map[ulid.ULID]*metadata.Meta
modified float64
name string
input map[ulid.ULID]*metadata.Meta
expected map[ulid.ULID]*metadata.Meta
modified float64
replicaLabelRemover *ReplicaLabelRemover
}{
{
name: "without replica labels",
Expand All @@ -898,7 +898,8 @@ func TestReplicaLabelRemover_Modify(t *testing.T) {
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
modified: 0,
modified: 0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"}),
},
{
name: "with replica labels",
Expand All @@ -914,11 +915,27 @@ func TestReplicaLabelRemover_Modify(t *testing.T) {
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(4): {Thanos: metadata.Thanos{Labels: map[string]string{"replica": "deduped"}}},
},
modified: 5.0,
modified: 5.0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{"replica", "rule_replica"}),
},
{
name: "no replica label specified in the ReplicaLabelRemover",
input: map[ulid.ULID]*metadata.Meta{
ULID(1): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
expected: map[ulid.ULID]*metadata.Meta{
ULID(1): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(2): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something"}}},
ULID(3): {Thanos: metadata.Thanos{Labels: map[string]string{"message": "something1"}}},
},
modified: 0,
replicaLabelRemover: NewReplicaLabelRemover(log.NewNopLogger(), []string{}),
},
} {
m := newTestFetcherMetrics()
testutil.Ok(t, rm.Modify(ctx, tcase.input, m.modified))
testutil.Ok(t, tcase.replicaLabelRemover.Modify(ctx, tcase.input, m.modified))

testutil.Equals(t, tcase.modified, promtest.ToFloat64(m.modified.WithLabelValues(replicaRemovedMeta)))
testutil.Equals(t, tcase.expected, tcase.input)
Expand Down

0 comments on commit 02672f3

Please sign in to comment.