diff --git a/CHANGELOG.md b/CHANGELOG.md index 969e6c8f3b5..b53819eb496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [BUGFIX] Ingester: Staleness markers written by the Ruler are no longer reported as discarded samples for the tenant. #7887 * [BUGFIX] Ruler: Stop retrying staleness markers that can never be accepted. A rule moved between rule groups left the original group re-appending staleness markers for its series on every evaluation until the ruler restarted. #7888 * [BUGFIX] Store Gateway: Fix the Parquet bucket store ignoring query sharding, which made every shard return all series. #7892 +* [BUGFIX] Querier: Fix the querier crashing instead of reporting the error when a blocks store set subservice fails. `blocksStoreReplicationSet` never initialised its `FailureWatcher`, so the failure callback dereferenced a nil pointer on the goroutine the service manager runs listeners on, and the resulting panic's stack trace buried the error logged for the actual cause. #7896 ## 1.22.0 in progress diff --git a/pkg/querier/blocks_store_replicated_set.go b/pkg/querier/blocks_store_replicated_set.go index d102a522e24..b2c1546a469 100644 --- a/pkg/querier/blocks_store_replicated_set.go +++ b/pkg/querier/blocks_store_replicated_set.go @@ -58,11 +58,12 @@ func newBlocksStoreReplicationSet( zoneStableShuffleSharding bool, ) (*blocksStoreReplicationSet, error) { s := &blocksStoreReplicationSet{ - storesRing: storesRing, - clientsPool: newStoreGatewayClientPool(client.NewRingServiceDiscovery(storesRing), clientConfig, logger, reg), - shardingStrategy: shardingStrategy, - balancingStrategy: balancingStrategy, - limits: limits, + storesRing: storesRing, + clientsPool: newStoreGatewayClientPool(client.NewRingServiceDiscovery(storesRing), clientConfig, logger, reg), + shardingStrategy: shardingStrategy, + balancingStrategy: balancingStrategy, + limits: limits, + subservicesWatcher: services.NewFailureWatcher(), zoneAwarenessEnabled: zoneAwarenessEnabled, zoneStableShuffleSharding: zoneStableShuffleSharding, diff --git a/pkg/querier/blocks_store_replicated_set_test.go b/pkg/querier/blocks_store_replicated_set_test.go index 6ccc65d4a82..9cb18073985 100644 --- a/pkg/querier/blocks_store_replicated_set_test.go +++ b/pkg/querier/blocks_store_replicated_set_test.go @@ -758,3 +758,30 @@ func getStoreGatewayClientAddrs(clients map[BlocksStoreClient][]ulid.ULID) map[s } return addrs } + +// TestBlocksStoreReplicationSet_SubservicesWatcherIsInitialised guards against a nil +// subservicesWatcher. starting() calls WatchManager on it, which registers a callback +// that sends on the watcher's channel, so a nil watcher panics in the goroutine the +// service manager runs the callback on as soon as a subservice fails. +func TestBlocksStoreReplicationSet_SubservicesWatcherIsInitialised(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger(), nil) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + require.NoError(t, ringStore.CAS(ctx, "test", func(in any) (any, bool, error) { + return ring.NewDesc(), true, nil + })) + + ringCfg := ring.Config{} + flagext.DefaultValues(&ringCfg) + + r, err := ring.NewWithStoreClientAndStrategy(ringCfg, "test", "test", ringStore, ring.NewIgnoreUnhealthyInstancesReplicationStrategy(), nil, nil) + require.NoError(t, err) + + s, err := newBlocksStoreReplicationSet(r, util.ShardingStrategyDefault, noLoadBalancing, &blocksStoreLimitsMock{}, ClientConfig{}, log.NewNopLogger(), prometheus.NewPedanticRegistry(), false, false) + require.NoError(t, err) + require.NotNil(t, s.subservicesWatcher) +}