From a4d9d3dfad5bb526ad4ef28bcbec06d2cade869f Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Tue, 7 Sep 2021 23:39:14 -0700 Subject: [PATCH 1/2] Using subring on getShardedRules when shuffle sharding Signed-off-by: Alan Protasio --- CHANGELOG.md | 1 + pkg/ruler/ruler.go | 12 +++++++++--- pkg/ruler/ruler_test.go | 13 ++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 284d3c1b69..2f6e080a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ * [BUGFIX] Querier: After query-frontend restart, querier may have lower than configured concurrency. #4417 * [BUGFIX] Memberlist: forward only changes, not entire original message. #4419 * [BUGFIX] Memberlist: don't accept old tombstones as incoming change, and don't forward such messages to other gossip members. #4420 +* [ENHANCEMENT] Rulers: Using shuffle sharding subring on GetRules API. #4466 ## 1.10.0 / 2021-08-03 diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index 1633df4ea7..a6335aca96 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -651,7 +651,7 @@ func (r *Ruler) GetRules(ctx context.Context) ([]*GroupStateDesc, error) { } if r.cfg.EnableSharding { - return r.getShardedRules(ctx) + return r.getShardedRules(ctx, userID) } return r.getLocalRules(userID) @@ -744,8 +744,14 @@ func (r *Ruler) getLocalRules(userID string) ([]*GroupStateDesc, error) { return groupDescs, nil } -func (r *Ruler) getShardedRules(ctx context.Context) ([]*GroupStateDesc, error) { - rulers, err := r.ring.GetReplicationSetForOperation(RingOp) +func (r *Ruler) getShardedRules(ctx context.Context, userID string) ([]*GroupStateDesc, error) { + ring := ring.ReadRing(r.ring) + + if shardSize := r.limits.RulerTenantShardSize(userID); shardSize > 0 && r.cfg.ShardingStrategy == util.ShardingStrategyShuffle { + ring = r.ring.ShuffleShard(userID, shardSize) + } + + rulers, err := ring.GetReplicationSetForOperation(RingOp) if err != nil { return nil, err } diff --git a/pkg/ruler/ruler_test.go b/pkg/ruler/ruler_test.go index bf28e7874c..04d8d3a2c4 100644 --- a/pkg/ruler/ruler_test.go +++ b/pkg/ruler/ruler_test.go @@ -427,11 +427,14 @@ func TestGetRules(t *testing.T) { require.NoError(t, err) require.Equal(t, len(allRulesByUser[u]), len(rules)) if tc.sharding { - mockPoolLClient := r.clientsPool.(*mockRulerClientsPool) - - // Right now we are calling all rules even with shuffle sharding - require.Equal(t, int32(len(rulerAddrMap)), mockPoolLClient.numberOfCalls.Load()) - mockPoolLClient.numberOfCalls.Store(0) + mockPoolClient := r.clientsPool.(*mockRulerClientsPool) + + if tc.shardingStrategy == util.ShardingStrategyShuffle { + require.Equal(t, int32(tc.shuffleShardSize), mockPoolClient.numberOfCalls.Load()) + } else { + require.Equal(t, int32(len(rulerAddrMap)), mockPoolClient.numberOfCalls.Load()) + } + mockPoolClient.numberOfCalls.Store(0) } }) } From 25280c855dc9fd9544e0c1380142665154631022 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Fri, 10 Sep 2021 12:46:13 -0700 Subject: [PATCH 2/2] Reording Changelog Signed-off-by: Alan Protasio --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f6e080a76..0faab6727d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ * [ENHANCEMENT] Updated Prometheus to include changes from prometheus/prometheus#9083. Now whenever `/labels` API calls include matchers, blocks store is queried for `LabelNames` with matchers instead of `Series` calls which was inefficient. #4380 * [ENHANCEMENT] Exemplars are now emitted for all gRPC calls and many operations tracked by histograms. #4462 * [ENHANCEMENT] New options `-server.http-listen-network` and `-server.grpc-listen-network` allow binding as 'tcp4' or 'tcp6'. #4462 +* [ENHANCEMENT] Rulers: Using shuffle sharding subring on GetRules API. #4466 * [BUGFIX] Fixes a panic in the query-tee when comparing result. #4465 * [BUGFIX] Frontend: Fixes @ modifier functions (start/end) when splitting queries by time. #4464 * [BUGFIX] Compactor: compactor will no longer try to compact blocks that are already marked for deletion. Previously compactor would consider blocks marked for deletion within `-compactor.deletion-delay / 2` period as eligible for compaction. #4328 @@ -50,7 +51,6 @@ * [BUGFIX] Querier: After query-frontend restart, querier may have lower than configured concurrency. #4417 * [BUGFIX] Memberlist: forward only changes, not entire original message. #4419 * [BUGFIX] Memberlist: don't accept old tombstones as incoming change, and don't forward such messages to other gossip members. #4420 -* [ENHANCEMENT] Rulers: Using shuffle sharding subring on GetRules API. #4466 ## 1.10.0 / 2021-08-03