Querier: initialise subservicesWatcher in blocksStoreReplicationSet

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

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,

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)
+}