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..8862fa31dfe 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,26 @@ 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) +}