Skip to content

Commit 3c89123

Browse files
authored
skip nil values in Memberlist WatchPrefix (#7429)
* skip nil values in Memberlist WatchPrefix Signed-off-by: SungJin1212 <tjdwls1201@gmail.com> * fix lint Signed-off-by: SungJin1212 <tjdwls1201@gmail.com> --------- Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
1 parent b150865 commit 3c89123

3 files changed

Lines changed: 66 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* [BUGFIX] Fix memory leak in `ReuseWriteRequestV2` by explicitly clearing the `Symbols` backing array string pointers before returning the object to `sync.Pool`. #7373
2020
* [BUGFIX] Distributor: Return HTTP 401 Unauthorized when tenant ID resolution fails in the Prometheus Remote Write 2.0 path. #7389
2121
* [BUGFIX] KV store: Fix false-positive `status_code="500"` metrics for HA tracker CAS operations when using memberlist. #7408
22+
* [BUGFIX] Memberlist: Skip nil values delivered by `WatchPrefix` when a key is deleted, preventing a panic in the HA tracker caused by a failed type assertion on a nil interface value. #7429
2223

2324
## 1.21.0 in progress
2425

pkg/ha/ha_tracker_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,66 @@ func TestHATracker_CleanupDeletesArePropagatedWithMemberlist(t *testing.T) {
223223
require.NotEmpty(t, broadcasts, "Cleanup Delete should generate a broadcast for tombstone propagation")
224224
}
225225

226+
// TestWatchPrefixNilPanicWithMemberlist reproduces the panic at ha_tracker.go:437:
227+
// With memberlist, WatchPrefix delivers a nil value when a key is deleted
228+
// (memberlist KV.get() returns nil for deleted/tombstone keys).
229+
func TestWatchPrefixNilPanicWithMemberlist(t *testing.T) {
230+
ctx := t.Context()
231+
logger := log.NewNopLogger()
232+
reg := prometheus.NewRegistry()
233+
234+
var kvCfg memberlist.KVConfig
235+
flagext.DefaultValues(&kvCfg)
236+
replicaDescCodec := GetReplicaDescCodec()
237+
kvCfg.Codecs = []codec.Codec{replicaDescCodec}
238+
239+
mkv := memberlist.NewKV(kvCfg, logger, &dnsProviderMock{}, reg)
240+
require.NoError(t, services.StartAndAwaitRunning(ctx, mkv))
241+
defer services.StopAndAwaitTerminated(ctx, mkv) //nolint:errcheck
242+
243+
client, err := memberlist.NewClient(mkv, replicaDescCodec)
244+
require.NoError(t, err)
245+
246+
trackerCfg := HATrackerConfig{
247+
EnableHATracker: false, // to inject our client before starting the tracker
248+
UpdateTimeout: time.Second,
249+
UpdateTimeoutJitterMax: 0,
250+
FailoverTimeout: 2 * time.Second,
251+
KVStore: kv.Config{Store: "memberlist"},
252+
}
253+
254+
tracker, err := NewHATracker(trackerCfg, nil, HATrackerStatusConfig{}, reg, "test", logger)
255+
require.NoError(t, err)
256+
tracker.cfg.EnableHATracker = true
257+
tracker.client = client
258+
259+
// Start the tracker — this starts the WatchPrefix loop in loop().
260+
require.NoError(t, services.StartAndAwaitRunning(ctx, tracker))
261+
defer services.StopAndAwaitTerminated(ctx, tracker) //nolint:errcheck
262+
263+
userID := "user1"
264+
cluster := "cluster1"
265+
replica := "replica0"
266+
key := userID + "/" + cluster
267+
268+
now := time.Now()
269+
require.NoError(t, tracker.CheckReplica(ctx, userID, cluster, replica, now))
270+
271+
test.Poll(t, 3*time.Second, nil, func() any {
272+
tracker.electedLock.RLock()
273+
defer tracker.electedLock.RUnlock()
274+
if _, ok := tracker.elected[key]; !ok {
275+
return fmt.Errorf("waiting for key to appear in elected cache")
276+
}
277+
return nil
278+
})
279+
280+
require.NoError(t, client.Delete(ctx, key))
281+
282+
time.Sleep(500 * time.Millisecond)
283+
require.Equal(t, services.Running, tracker.State(), "HATracker should still be running after receiving nil value from memberlist WatchPrefix")
284+
}
285+
226286
// Test that values are set in the HATracker after WatchPrefix has found it in the KVStore.
227287
func TestWatchPrefixAssignment(t *testing.T) {
228288
t.Parallel()

pkg/ring/kv/memberlist/memberlist_client.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,11 @@ func (m *KV) WatchPrefix(ctx context.Context, prefix string, codec codec.Codec,
853853
continue
854854
}
855855

856+
if val == nil {
857+
// Skip nil that can be returned when the key is deleted.
858+
continue
859+
}
860+
856861
if !f(key, val) {
857862
return
858863
}

0 commit comments

Comments
 (0)