From 4badf71067507b94782f6962d3b176d7b9803655 Mon Sep 17 00:00:00 2001 From: Goutham Annem Date: Sat, 18 Jul 2026 15:09:09 -0700 Subject: [PATCH 1/2] fix(distributor): allow UserStats to succeed when a ring member is LEAVING UserStats queried all ingesters via GetIngestersForMetadata then forced MaxErrors = 0, requiring every ingester to respond successfully. When any ingester was in the LEAVING state (normal during a rolling update) and its RPC failed, UserStats returned an error to the caller. Remove the MaxErrors = 0 override so the ring's own quorum tolerance (RF/2 failures allowed for a Read operation) applies. Partial stats are better than a hard failure on the /api/v1/user_stats page. Closes #4936 Signed-off-by: Goutham Annem --- CHANGELOG.md | 1 + pkg/distributor/distributor.go | 6 +++-- pkg/distributor/distributor_test.go | 34 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c9fd986ee3..cbf7e4688d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## master / unreleased +* [BUGFIX] Distributor: `UserStats` no longer returns an error when one ingester in the replication set is LEAVING or otherwise temporarily unavailable. #4936 * [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7160 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446 * [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. #7481 diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go index edf9b750b87..7d2fe1dd0fa 100644 --- a/pkg/distributor/distributor.go +++ b/pkg/distributor/distributor.go @@ -1687,8 +1687,10 @@ func (d *Distributor) UserStats(ctx context.Context) (*ingester.UserStats, error return nil, err } - // Make sure we get a successful response from all of them. - replicationSet.MaxErrors = 0 + // Allow a subset of ingesters to fail (e.g. during a rolling update where + // some are LEAVING). MaxErrors is already set by the ring to the quorum + // tolerance; overriding it to 0 caused the endpoint to error whenever a + // LEAVING ingester was in the set and returned an error (issue #4936). req := &ingester_client.UserStatsRequest{} resps, err := d.ForReplicationSet(ctx, replicationSet, false, false, func(ctx context.Context, client ingester_client.IngesterClient) (any, error) { diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index 2cb0b1522f3..e9950994a83 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -4070,6 +4070,16 @@ func (s *metricsForLabelMatchersStream) Recv() (*client.MetricsForLabelMatchersS return result, nil } +func (i *mockIngester) UserStats(ctx context.Context, in *client.UserStatsRequest, opts ...grpc.CallOption) (*client.UserStatsResponse, error) { + if !i.happy.Load() { + return nil, errFail + } + return &client.UserStatsResponse{ + IngestionRate: 0, + NumSeries: 0, + }, nil +} + func (i *mockIngester) AllUserStats(ctx context.Context, in *client.UserStatsRequest, opts ...grpc.CallOption) (*client.UsersStatsResponse, error) { return &i.stats, nil } @@ -4971,3 +4981,27 @@ func TestDistributor_ReceivedHistogramBucketsMetric(t *testing.T) { // GenerateTestHistogram(0): 4 positive + 4 negative + 1 zero = 9 buckets require.Equal(t, float64(9), m.GetHistogram().GetSampleSum()) } + + +// TestUserStats_ToleratesLeavingIngester verifies that UserStats succeeds +// when one of the ring ingesters fails (e.g. because it is LEAVING). +// Before the fix, MaxErrors was set to 0, causing the endpoint to return an +// error whenever a single ingester in the replication set was unavailable. +func TestUserStats_ToleratesLeavingIngester(t *testing.T) { + t.Parallel() + + // 3 ingesters, RF=3, only 2 are happy — simulates one LEAVING ingester + // returning an error. + ctx := user.InjectOrgID(context.Background(), "user1") + distributors, _, _, _ := prepare(t, prepConfig{ + numIngesters: 3, + happyIngesters: 2, + numDistributors: 1, + replicationFactor: 3, + }) + + // UserStats must succeed despite one ingester failing. + stats, err := distributors[0].UserStats(ctx) + require.NoError(t, err) + require.NotNil(t, stats) +} From f682c1952e9be2ecf130a19bc39854bd88d8c0ae Mon Sep 17 00:00:00 2001 From: Goutham Annem Date: Wed, 22 Jul 2026 00:13:38 -0700 Subject: [PATCH 2/2] fix(lint): apply gofmt to distributor_test.go Signed-off-by: Goutham Annem --- pkg/distributor/distributor_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index e9950994a83..8862fa31dfe 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -4982,7 +4982,6 @@ func TestDistributor_ReceivedHistogramBucketsMetric(t *testing.T) { require.Equal(t, float64(9), m.GetHistogram().GetSampleSum()) } - // TestUserStats_ToleratesLeavingIngester verifies that UserStats succeeds // when one of the ring ingesters fails (e.g. because it is LEAVING). // Before the fix, MaxErrors was set to 0, causing the endpoint to return an @@ -4994,9 +4993,9 @@ func TestUserStats_ToleratesLeavingIngester(t *testing.T) { // returning an error. ctx := user.InjectOrgID(context.Background(), "user1") distributors, _, _, _ := prepare(t, prepConfig{ - numIngesters: 3, - happyIngesters: 2, - numDistributors: 1, + numIngesters: 3, + happyIngesters: 2, + numDistributors: 1, replicationFactor: 3, })