Skip to content

Commit

Permalink
Fix cmd info race. Fixes redis#578
Browse files Browse the repository at this point in the history
  • Loading branch information
vmihailenco committed Jun 17, 2017
1 parent eb06603 commit 5132e15
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 18 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ matrix:
- go: tip

install:
- go get go4.org/syncutil
- go get github.com/onsi/ginkgo
- go get github.com/onsi/gomega
38 changes: 27 additions & 11 deletions cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"sync/atomic"
"time"

"go4.org/syncutil"

"github.com/go-redis/redis/internal"
"github.com/go-redis/redis/internal/hashtag"
"github.com/go-redis/redis/internal/pool"
Expand Down Expand Up @@ -335,10 +337,12 @@ type ClusterClient struct {
cmdable

opt *ClusterOptions
cmds map[string]*CommandInfo
nodes *clusterNodes
_state atomic.Value

cmdsInfoOnce syncutil.Once
cmdsInfo map[string]*CommandInfo

// Reports where slots reloading is in progress.
reloading uint32
}
Expand Down Expand Up @@ -389,13 +393,34 @@ func (c *ClusterClient) state() *clusterState {
return nil
}

func (c *ClusterClient) cmdInfo(name string) *CommandInfo {
err := c.cmdsInfoOnce.Do(func() error {
node, err := c.nodes.Random()
if err != nil {
return err
}

cmdsInfo, err := node.Client.Command().Result()
if err != nil {
return err
}

c.cmdsInfo = cmdsInfo
return nil
})
if err != nil {
return nil
}
return c.cmdsInfo[name]
}

func (c *ClusterClient) cmdSlotAndNode(state *clusterState, cmd Cmder) (int, *clusterNode, error) {
if state == nil {
node, err := c.nodes.Random()
return 0, node, err
}

cmdInfo := c.cmds[cmd.Name()]
cmdInfo := c.cmdInfo(cmd.Name())
firstKey := cmd.arg(cmdFirstKeyPos(cmd, cmdInfo))
slot := hashtag.Slot(firstKey)

Expand Down Expand Up @@ -631,15 +656,6 @@ func (c *ClusterClient) reloadSlots() (*clusterState, error) {
return nil, err
}

// TODO: fix race
if c.cmds == nil {
cmds, err := node.Client.Command().Result()
if err != nil {
return nil, err
}
c.cmds = cmds
}

slots, err := node.Client.ClusterSlots().Result()
if err != nil {
return nil, err
Expand Down
18 changes: 11 additions & 7 deletions ring.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"sync/atomic"
"time"

"go4.org/syncutil"

"github.com/go-redis/redis/internal"
"github.com/go-redis/redis/internal/consistenthash"
"github.com/go-redis/redis/internal/hashtag"
Expand Down Expand Up @@ -134,7 +136,7 @@ type Ring struct {
hash *consistenthash.Map
shards map[string]*ringShard

cmdsInfoOnce *sync.Once
cmdsInfoOnce syncutil.Once
cmdsInfo map[string]*CommandInfo

closed bool
Expand All @@ -149,8 +151,6 @@ func NewRing(opt *RingOptions) *Ring {

hash: consistenthash.New(nreplicas, nil),
shards: make(map[string]*ringShard),

cmdsInfoOnce: new(sync.Once),
}
ring.setProcessor(ring.Process)
for name, addr := range opt.Addrs {
Expand Down Expand Up @@ -242,17 +242,21 @@ func (c *Ring) ForEachShard(fn func(client *Client) error) error {
}

func (c *Ring) cmdInfo(name string) *CommandInfo {
c.cmdsInfoOnce.Do(func() {
err := c.cmdsInfoOnce.Do(func() error {
var firstErr error
for _, shard := range c.shards {
cmdsInfo, err := shard.Client.Command().Result()
if err == nil {
c.cmdsInfo = cmdsInfo
return
return nil
}
if firstErr == nil {
firstErr = err
}
}
c.cmdsInfoOnce = &sync.Once{}
return firstErr
})
if c.cmdsInfo == nil {
if err != nil {
return nil
}
return c.cmdsInfo[name]
Expand Down

0 comments on commit 5132e15

Please sign in to comment.