diff --git a/group.go b/group.go index 57c369a..ae3d79b 100644 --- a/group.go +++ b/group.go @@ -459,18 +459,16 @@ func (g *group) RemoveKeys(ctx context.Context, keys ...string) error { g.Stats.RemovedKeys.Add(int64(len(keys))) keysByOwner := make(map[peer.Client][]string) - var localKeys []string for _, key := range keys { - owner, isRemote := g.instance.PickPeer(key) - if isRemote { + if owner, isRemote := g.instance.PickPeer(key); isRemote { keysByOwner[owner] = append(keysByOwner[owner], key) - } else { - localKeys = append(localKeys, key) } } - for _, key := range localKeys { + // A non-owner may hold the key in its hot cache, so clear every key here + // whether or not an owner request later fails. + for _, key := range keys { g.LocalRemove(key) } diff --git a/remove_keys_test.go b/remove_keys_test.go index e59003f..da2390f 100644 --- a/remove_keys_test.go +++ b/remove_keys_test.go @@ -19,6 +19,7 @@ package groupcache_test import ( "context" "fmt" + "sync" "testing" "time" @@ -82,6 +83,52 @@ func TestRemoveKeys(t *testing.T) { } } +func TestRemoveKeysClearsCallerCopies(t *testing.T) { + ctx := context.Background() + + err := cluster.Start(ctx, 3, groupcache.Options{}) + require.NoError(t, err) + defer func() { _ = cluster.Shutdown(ctx) }() + + var mu sync.Mutex + loads := make(map[string]int) + getter := groupcache.GetterFunc(func(ctx context.Context, key string, dest transport.Sink) error { + mu.Lock() + loads[key]++ + mu.Unlock() + return dest.SetString(fmt.Sprintf("value-%s", key), time.Now().Add(time.Minute*5)) + }) + + var group transport.Group + for i := 0; i < 3; i++ { + g, err := cluster.DaemonAt(i).NewGroup("test-remove-keys-caller", 3000000, getter) + require.NoError(t, err) + if i == 0 { + group = g + } + } + + // Keys a-e are spread across the three daemons, so some are owned by peers other than daemon 0. + keys := []string{"a", "b", "c", "d", "e"} + for _, key := range keys { + var value string + require.NoError(t, group.Get(ctx, key, transport.StringSink(&value))) + } + + require.NoError(t, group.RemoveKeys(ctx, keys...)) + + for _, key := range keys { + var value string + require.NoError(t, group.Get(ctx, key, transport.StringSink(&value))) + } + + mu.Lock() + defer mu.Unlock() + for _, key := range keys { + assert.Equal(t, 2, loads[key], key) + } +} + func TestRemoveKeysEmpty(t *testing.T) { ctx := context.Background()