Skip to content

Commit e0fbcea

Browse files
committed
Set correct cluster slot for scan commands, similarly to Java's Jedis client
- 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?
1 parent b1103e3 commit e0fbcea

File tree

3 files changed

+53
-0
lines changed

3 files changed

+53
-0
lines changed

commands.go

+16
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
"github.com/redis/go-redis/v9/internal"
14+
"github.com/redis/go-redis/v9/internal/hashtag"
1415
)
1516

1617
// KeepTTL is a Redis KEEPTTL option to keep existing TTL, it requires your redis-server version >= 6.0,
@@ -1353,6 +1354,9 @@ func (c cmdable) Scan(ctx context.Context, cursor uint64, match string, count in
13531354
args = append(args, "count", count)
13541355
}
13551356
cmd := NewScanCmd(ctx, c, args...)
1357+
if hashtag.Present(match) {
1358+
cmd.SetFirstKeyPos(3)
1359+
}
13561360
_ = c(ctx, cmd)
13571361
return cmd
13581362
}
@@ -1369,6 +1373,9 @@ func (c cmdable) ScanType(ctx context.Context, cursor uint64, match string, coun
13691373
args = append(args, "type", keyType)
13701374
}
13711375
cmd := NewScanCmd(ctx, c, args...)
1376+
if hashtag.Present(match) {
1377+
cmd.SetFirstKeyPos(3)
1378+
}
13721379
_ = c(ctx, cmd)
13731380
return cmd
13741381
}
@@ -1382,6 +1389,9 @@ func (c cmdable) SScan(ctx context.Context, key string, cursor uint64, match str
13821389
args = append(args, "count", count)
13831390
}
13841391
cmd := NewScanCmd(ctx, c, args...)
1392+
if hashtag.Present(match) {
1393+
cmd.SetFirstKeyPos(4)
1394+
}
13851395
_ = c(ctx, cmd)
13861396
return cmd
13871397
}
@@ -1395,6 +1405,9 @@ func (c cmdable) HScan(ctx context.Context, key string, cursor uint64, match str
13951405
args = append(args, "count", count)
13961406
}
13971407
cmd := NewScanCmd(ctx, c, args...)
1408+
if hashtag.Present(match) {
1409+
cmd.SetFirstKeyPos(4)
1410+
}
13981411
_ = c(ctx, cmd)
13991412
return cmd
14001413
}
@@ -1408,6 +1421,9 @@ func (c cmdable) ZScan(ctx context.Context, key string, cursor uint64, match str
14081421
args = append(args, "count", count)
14091422
}
14101423
cmd := NewScanCmd(ctx, c, args...)
1424+
if hashtag.Present(match) {
1425+
cmd.SetFirstKeyPos(4)
1426+
}
14111427
_ = c(ctx, cmd)
14121428
return cmd
14131429
}

internal/hashtag/hashtag.go

+12
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ func Key(key string) string {
5656
return key
5757
}
5858

59+
func Present(key string) bool {
60+
if key == "" {
61+
return false
62+
}
63+
if s := strings.IndexByte(key, '{'); s > -1 {
64+
if e := strings.IndexByte(key[s+1:], '}'); e > 0 {
65+
return true
66+
}
67+
}
68+
return false
69+
}
70+
5971
func RandomSlot() int {
6072
return rand.Intn(slotNumber)
6173
}

internal/hashtag/hashtag_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,28 @@ var _ = Describe("HashSlot", func() {
6969
}
7070
})
7171
})
72+
73+
var _ = Describe("Present", func() {
74+
It("should calculate hash slots", func() {
75+
tests := []struct {
76+
key string
77+
present bool
78+
}{
79+
{"123456789", false},
80+
{"{}foo", false},
81+
{"foo{}", false},
82+
{"foo{}{bar}", false},
83+
{"", false},
84+
{string([]byte{83, 153, 134, 118, 229, 214, 244, 75, 140, 37, 215, 215}), false},
85+
{"foo{bar}", true},
86+
{"{foo}bar", true},
87+
{"{user1000}.following", true},
88+
{"foo{{bar}}zap", true},
89+
{"foo{bar}{zap}", true},
90+
}
91+
92+
for _, test := range tests {
93+
Expect(Present(test.key)).To(Equal(test.present), "for %s", test.key)
94+
}
95+
})
96+
})

0 commit comments

Comments
 (0)