Skip to content

Commit

Permalink
Add new parameter to the RingOptions to override CommandInfo first key
Browse files Browse the repository at this point in the history
There is problem with `eval` and `evalsha` commands.
`COMMAND INFO eval` returns first key position equals `0`.
After that, redis ring chooses `eval` as a value for sharding.
They, if you try to delete created value, ring may choose another shard
and delete won't work.

Eval command should be parsed, to be sharded properly, according to
redis specs: http://redis.io/commands/command .

I've introduced a new flag in the `RingOptions`, which will enable new
behavior: `EnableKeyLocationParsing`.

If it is enabled, `cmdFirstKey` will try to get key position using
`cmd.getFirstKeyPos()`. This function is defined for `eval` and
`evalsha` commands.
If it has parameters, it will return `3`, otherwise it will return `0`.
  • Loading branch information
Artem Chernyshev committed Oct 4, 2016
1 parent 5a272d0 commit 03da66c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
1 change: 1 addition & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func redisRingOptions() *redis.RingOptions {
PoolTimeout: 30 * time.Second,
IdleTimeout: 500 * time.Millisecond,
IdleCheckFrequency: 500 * time.Millisecond,
RouteByEvalKeys: true,
}
}

Expand Down
21 changes: 21 additions & 0 deletions ring.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ type RingOptions struct {
PoolTimeout time.Duration
IdleTimeout time.Duration
IdleCheckFrequency time.Duration

// RouteByEvalKeys flag to enable eval and evalsha key position parsing for sharding
RouteByEvalKeys bool
}

func (opt *RingOptions) init() {
Expand Down Expand Up @@ -132,6 +135,8 @@ type Ring struct {
cmdsInfoOnce *sync.Once

closed bool

routeByEvalKeys bool
}

var _ Cmdable = (*Ring)(nil)
Expand All @@ -154,6 +159,7 @@ func NewRing(opt *RingOptions) *Ring {
clopt.Addr = addr
ring.addClient(name, NewClient(clopt))
}
ring.routeByEvalKeys = opt.RouteByEvalKeys
go ring.heartbeat()
return ring
}
Expand Down Expand Up @@ -221,7 +227,22 @@ func (c *Ring) cmdInfo(name string) *CommandInfo {
return c.cmdsInfo[name]
}

func (c *Ring) getEvalFirstKey(cmd Cmder) string {
if c.routeByEvalKeys && cmd.arg(2) != "0" {
return cmd.arg(3)
} else {
return cmd.arg(0)
}
}

func (c *Ring) cmdFirstKey(cmd Cmder) string {
switch cmd.arg(0) {
case "eval":
return c.getEvalFirstKey(cmd)
case "evalsha":
return c.getEvalFirstKey(cmd)
}

cmdInfo := c.cmdInfo(cmd.arg(0))
if cmdInfo == nil {
internal.Logf("info for cmd=%s not found", cmd.arg(0))
Expand Down
17 changes: 17 additions & 0 deletions ring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,23 @@ var _ = Describe("Redis Ring", func() {
Expect(ringShard2.Info().Val()).To(ContainSubstring("keys=100"))
})

It("supports eval key search", func() {
script := redis.NewScript(`
local r = redis.call('SET', KEYS[1], ARGV[1])
return r
`)

var key string
for i := 0; i < 100; i++ {
key = fmt.Sprintf("key{%d}", i)
err := script.Run(ring, []string{key}, "value").Err()
Expect(err).NotTo(HaveOccurred())
}

Expect(ringShard1.Info().Val()).To(ContainSubstring("keys=52"))
Expect(ringShard2.Info().Val()).To(ContainSubstring("keys=48"))
})

Describe("pipelining", func() {
It("returns an error when all shards are down", func() {
ring := redis.NewRing(&redis.RingOptions{})
Expand Down

0 comments on commit 03da66c

Please sign in to comment.