Skip to content

Commit

Permalink
Set correct cluster slot for scan commands, similarly to Java's Jedis…
Browse files Browse the repository at this point in the history
… 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.

- 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?
  • Loading branch information
pete-woods committed Jun 12, 2023
1 parent 0bdc7dd commit 36a52f2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
16 changes: 16 additions & 0 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/redis/go-redis/v9/internal"
"github.com/redis/go-redis/v9/internal/hashtag"
)

// KeepTTL is a Redis KEEPTTL option to keep existing TTL, it requires your redis-server version >= 6.0,
Expand Down Expand Up @@ -1353,6 +1354,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
}
Expand All @@ -1369,6 +1373,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
}
Expand All @@ -1382,6 +1389,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
}
Expand All @@ -1395,6 +1405,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
}
Expand All @@ -1408,6 +1421,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
}
Expand Down
12 changes: 12 additions & 0 deletions internal/hashtag/hashtag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
27 changes: 27 additions & 0 deletions internal/hashtag/hashtag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,30 @@ 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},
}
// Empty keys receive random slot.
rand.Seed(100)

for _, test := range tests {
Expect(Present(test.key)).To(Equal(test.present), "for %s", test.key)
}
})
})

0 comments on commit 36a52f2

Please sign in to comment.