From 4ac591c7c4ae10e703f5e08bbe93462fd5371678 Mon Sep 17 00:00:00 2001 From: Pete Woods Date: Tue, 24 Jun 2025 11:43:03 +0100 Subject: [PATCH] Set correct cluster slot for scan commands, similarly to Java's Jedis client (#2623) - At present, the `scan` command is dispatched to a random slot. - As far as I can tell, the scanX family of commands are not cluster aware (e.g. don't redirect the client to the correct slot). - You can see [here](https://github.com/redis/jedis/blob/869dc0bb6625b85c8bf15bf1361bde485a304338/src/main/java/redis/clients/jedis/ShardedCommandObjects.java#L101), the Jedis client calling `processKey` on the match argument, and this is what this PR also does. We've had this patch running in production, and it seems to work well for us. For further thought: - Continuing looking at other Redis clients (e.g. Jedis), they outright [reject as invalid](https://github.com/redis/jedis/blob/869dc0bb6625b85c8bf15bf1361bde485a304338/src/main/java/redis/clients/jedis/ShardedCommandObjects.java#L98) any scan command that does not include a hash-tag. Presumably this has the advantage of users not being surprised when their scan produces no results when a random server is picked. - Perhaps it would be sensible for go-redis to do the same also? Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> --- generic_commands.go | 8 ++++++++ hash_commands.go | 8 ++++++++ internal/hashtag/hashtag.go | 12 ++++++++++++ internal/hashtag/hashtag_test.go | 25 +++++++++++++++++++++++++ set_commands.go | 9 ++++++++- sortedset_commands.go | 5 +++++ 6 files changed, 66 insertions(+), 1 deletion(-) diff --git a/generic_commands.go b/generic_commands.go index dc6c3fe0..c7100222 100644 --- a/generic_commands.go +++ b/generic_commands.go @@ -3,6 +3,8 @@ package redis import ( "context" "time" + + "github.com/redis/go-redis/v9/internal/hashtag" ) type GenericCmdable interface { @@ -363,6 +365,9 @@ func (c cmdable) Scan(ctx context.Context, cursor uint64, match string, count in args = append(args, "count", count) } cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(3) + } _ = c(ctx, cmd) return cmd } @@ -379,6 +384,9 @@ func (c cmdable) ScanType(ctx context.Context, cursor uint64, match string, coun args = append(args, "type", keyType) } cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(3) + } _ = c(ctx, cmd) return cmd } diff --git a/hash_commands.go b/hash_commands.go index 98a361b3..335cb950 100644 --- a/hash_commands.go +++ b/hash_commands.go @@ -3,6 +3,8 @@ package redis import ( "context" "time" + + "github.com/redis/go-redis/v9/internal/hashtag" ) type HashCmdable interface { @@ -192,6 +194,9 @@ func (c cmdable) HScan(ctx context.Context, key string, cursor uint64, match str args = append(args, "count", count) } cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(4) + } _ = c(ctx, cmd) return cmd } @@ -211,6 +216,9 @@ func (c cmdable) HScanNoValues(ctx context.Context, key string, cursor uint64, m } args = append(args, "novalues") cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(4) + } _ = c(ctx, cmd) return cmd } diff --git a/internal/hashtag/hashtag.go b/internal/hashtag/hashtag.go index f13ee816..ea56fd6c 100644 --- a/internal/hashtag/hashtag.go +++ b/internal/hashtag/hashtag.go @@ -56,6 +56,18 @@ func Key(key string) string { return key } +func Present(key string) bool { + if key == "" { + return false + } + if s := strings.IndexByte(key, '{'); s > -1 { + if e := strings.IndexByte(key[s+1:], '}'); e > 0 { + return true + } + } + return false +} + func RandomSlot() int { return rand.Intn(slotNumber) } diff --git a/internal/hashtag/hashtag_test.go b/internal/hashtag/hashtag_test.go index fe4865b7..983e3928 100644 --- a/internal/hashtag/hashtag_test.go +++ b/internal/hashtag/hashtag_test.go @@ -69,3 +69,28 @@ var _ = Describe("HashSlot", func() { } }) }) + +var _ = Describe("Present", func() { + It("should calculate hash slots", func() { + tests := []struct { + key string + present bool + }{ + {"123456789", false}, + {"{}foo", false}, + {"foo{}", false}, + {"foo{}{bar}", false}, + {"", false}, + {string([]byte{83, 153, 134, 118, 229, 214, 244, 75, 140, 37, 215, 215}), false}, + {"foo{bar}", true}, + {"{foo}bar", true}, + {"{user1000}.following", true}, + {"foo{{bar}}zap", true}, + {"foo{bar}{zap}", true}, + } + + for _, test := range tests { + Expect(Present(test.key)).To(Equal(test.present), "for %s", test.key) + } + }) +}) diff --git a/set_commands.go b/set_commands.go index 355f514a..79efa6e4 100644 --- a/set_commands.go +++ b/set_commands.go @@ -1,6 +1,10 @@ package redis -import "context" +import ( + "context" + + "github.com/redis/go-redis/v9/internal/hashtag" +) type SetCmdable interface { SAdd(ctx context.Context, key string, members ...interface{}) *IntCmd @@ -211,6 +215,9 @@ func (c cmdable) SScan(ctx context.Context, key string, cursor uint64, match str args = append(args, "count", count) } cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(4) + } _ = c(ctx, cmd) return cmd } diff --git a/sortedset_commands.go b/sortedset_commands.go index 14b35858..e48e7367 100644 --- a/sortedset_commands.go +++ b/sortedset_commands.go @@ -4,6 +4,8 @@ import ( "context" "strings" "time" + + "github.com/redis/go-redis/v9/internal/hashtag" ) type SortedSetCmdable interface { @@ -719,6 +721,9 @@ func (c cmdable) ZScan(ctx context.Context, key string, cursor uint64, match str args = append(args, "count", count) } cmd := NewScanCmd(ctx, c, args...) + if hashtag.Present(match) { + cmd.SetFirstKeyPos(4) + } _ = c(ctx, cmd) return cmd }