From f8ea5495bafcdcb83589d2eef1953c0674371885 Mon Sep 17 00:00:00 2001 From: ofekshenawa <ofek.shenawa@redis.com> Date: Wed, 29 Nov 2023 09:56:41 +0200 Subject: [PATCH 1/3] remove command command from oss cluster --- osscluster.go | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/osscluster.go b/osscluster.go index 93e0eef1e..3c4d3d6fb 100644 --- a/osscluster.go +++ b/osscluster.go @@ -861,7 +861,7 @@ func NewClusterClient(opt *ClusterOptions) *ClusterClient { } c.state = newClusterStateHolder(c.loadState) - c.cmdsInfoCache = newCmdsInfoCache(c.cmdsInfo) + // c.cmdsInfoCache = newCmdsInfoCache(c.cmdsInfo) c.cmdable = c.Process c.initHooks(hooks{ @@ -907,7 +907,7 @@ func (c *ClusterClient) Process(ctx context.Context, cmd Cmder) error { } func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error { - cmdInfo := c.cmdInfo(ctx, cmd.Name()) + // cmdInfo := c.cmdInfo(ctx, cmd.Name()) slot := c.cmdSlot(ctx, cmd) var node *clusterNode var ask bool @@ -921,7 +921,7 @@ func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error { if node == nil { var err error - node, err = c.cmdNode(ctx, cmdInfo, slot) + node, err = c.cmdNode(ctx, slot) if err != nil { return err } @@ -1252,7 +1252,7 @@ func (c *ClusterClient) mapCmdsByNode(ctx context.Context, cmdsMap *cmdsMap, cmd return err } - if c.opt.ReadOnly && c.cmdsAreReadOnly(ctx, cmds) { + if c.opt.ReadOnly { for _, cmd := range cmds { slot := c.cmdSlot(ctx, cmd) node, err := c.slotReadOnlyNode(state, slot) @@ -1275,15 +1275,15 @@ func (c *ClusterClient) mapCmdsByNode(ctx context.Context, cmdsMap *cmdsMap, cmd return nil } -func (c *ClusterClient) cmdsAreReadOnly(ctx context.Context, cmds []Cmder) bool { - for _, cmd := range cmds { - cmdInfo := c.cmdInfo(ctx, cmd.Name()) - if cmdInfo == nil || !cmdInfo.ReadOnly { - return false - } - } - return true -} +// func (c *ClusterClient) cmdsAreReadOnly(ctx context.Context, cmds []Cmder) bool { +// for _, cmd := range cmds { +// cmdInfo := c.cmdInfo(ctx, cmd.Name()) +// if cmdInfo == nil || !cmdInfo.ReadOnly { +// return false +// } +// } +// return true +// } func (c *ClusterClient) processPipelineNode( ctx context.Context, node *clusterNode, cmds []Cmder, failedCmds *cmdsMap, @@ -1778,13 +1778,13 @@ func (c *ClusterClient) cmdInfo(ctx context.Context, name string) *CommandInfo { } func (c *ClusterClient) cmdSlot(ctx context.Context, cmd Cmder) int { - args := cmd.Args() - if args[0] == "cluster" && args[1] == "getkeysinslot" { - return args[2].(int) - } + // args := cmd.Args() + // if args[0] == "cluster" && args[1] == "getkeysinslot" { + // return args[2].(int) + // } - cmdInfo := c.cmdInfo(ctx, cmd.Name()) - return cmdSlot(cmd, cmdFirstKeyPos(cmd, cmdInfo)) + // cmdInfo := c.cmdInfo(ctx, cmd.Name()) + return cmdSlot(cmd, cmdFirstKeyPos(cmd, nil)) } func cmdSlot(cmd Cmder, pos int) int { @@ -1797,7 +1797,6 @@ func cmdSlot(cmd Cmder, pos int) int { func (c *ClusterClient) cmdNode( ctx context.Context, - cmdInfo *CommandInfo, slot int, ) (*clusterNode, error) { state, err := c.state.Get(ctx) @@ -1805,7 +1804,7 @@ func (c *ClusterClient) cmdNode( return nil, err } - if c.opt.ReadOnly && cmdInfo != nil && cmdInfo.ReadOnly { + if c.opt.ReadOnly { return c.slotReadOnlyNode(state, slot) } return state.slotMasterNode(slot) From 23d4fc4698711611a72f1bdf590d923c80a679e4 Mon Sep 17 00:00:00 2001 From: ofekshenawa <ofek.shenawa@redis.com> Date: Wed, 29 Nov 2023 16:03:07 +0200 Subject: [PATCH 2/3] remove command command from oss cluster --- bench_decode_test.go | 8 -------- command.go | 6 +----- osscluster.go | 42 ++++++++++++++++++++++-------------------- ring.go | 6 ++---- 4 files changed, 25 insertions(+), 37 deletions(-) diff --git a/bench_decode_test.go b/bench_decode_test.go index 50e339086..16bdf2cd3 100644 --- a/bench_decode_test.go +++ b/bench_decode_test.go @@ -59,14 +59,6 @@ func NewClusterClientStub(resp []byte) *ClientStub { }, }) - // init command. - tmpClient := NewClient(&Options{Addr: ":6379"}) - cmdsInfo, err := tmpClient.Command(ctx).Result() - _ = tmpClient.Close() - client.cmdsInfoCache = newCmdsInfoCache(func(_ context.Context) (map[string]*CommandInfo, error) { - return cmdsInfo, err - }) - stub.Cmdable = client return stub } diff --git a/command.go b/command.go index c641a3fae..3e1e51768 100644 --- a/command.go +++ b/command.go @@ -62,7 +62,7 @@ func writeCmd(wr *proto.Writer, cmd Cmder) error { return wr.WriteArgs(cmd.Args()) } -func cmdFirstKeyPos(cmd Cmder, info *CommandInfo) int { +func cmdFirstKeyPos(cmd Cmder) int { if pos := cmd.firstKeyPos(); pos != 0 { return int(pos) } @@ -82,10 +82,6 @@ func cmdFirstKeyPos(cmd Cmder, info *CommandInfo) int { return 2 } } - - if info != nil { - return int(info.FirstKeyPos) - } return 1 } diff --git a/osscluster.go b/osscluster.go index 3c4d3d6fb..b8a82d9bc 100644 --- a/osscluster.go +++ b/osscluster.go @@ -861,7 +861,7 @@ func NewClusterClient(opt *ClusterOptions) *ClusterClient { } c.state = newClusterStateHolder(c.loadState) - // c.cmdsInfoCache = newCmdsInfoCache(c.cmdsInfo) + c.cmdsInfoCache = newCmdsInfoCache(c.cmdsInfo) c.cmdable = c.Process c.initHooks(hooks{ @@ -907,7 +907,6 @@ func (c *ClusterClient) Process(ctx context.Context, cmd Cmder) error { } func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error { - // cmdInfo := c.cmdInfo(ctx, cmd.Name()) slot := c.cmdSlot(ctx, cmd) var node *clusterNode var ask bool @@ -921,7 +920,7 @@ func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error { if node == nil { var err error - node, err = c.cmdNode(ctx, slot) + node, err = c.cmdNode(ctx, cmd.Name(), slot) if err != nil { return err } @@ -1252,7 +1251,7 @@ func (c *ClusterClient) mapCmdsByNode(ctx context.Context, cmdsMap *cmdsMap, cmd return err } - if c.opt.ReadOnly { + if c.opt.ReadOnly && c.cmdsAreReadOnly(ctx, cmds) { for _, cmd := range cmds { slot := c.cmdSlot(ctx, cmd) node, err := c.slotReadOnlyNode(state, slot) @@ -1275,15 +1274,15 @@ func (c *ClusterClient) mapCmdsByNode(ctx context.Context, cmdsMap *cmdsMap, cmd return nil } -// func (c *ClusterClient) cmdsAreReadOnly(ctx context.Context, cmds []Cmder) bool { -// for _, cmd := range cmds { -// cmdInfo := c.cmdInfo(ctx, cmd.Name()) -// if cmdInfo == nil || !cmdInfo.ReadOnly { -// return false -// } -// } -// return true -// } +func (c *ClusterClient) cmdsAreReadOnly(ctx context.Context, cmds []Cmder) bool { + for _, cmd := range cmds { + cmdInfo := c.cmdInfo(ctx, cmd.Name()) + if cmdInfo == nil || !cmdInfo.ReadOnly { + return false + } + } + return true +} func (c *ClusterClient) processPipelineNode( ctx context.Context, node *clusterNode, cmds []Cmder, failedCmds *cmdsMap, @@ -1778,13 +1777,12 @@ func (c *ClusterClient) cmdInfo(ctx context.Context, name string) *CommandInfo { } func (c *ClusterClient) cmdSlot(ctx context.Context, cmd Cmder) int { - // args := cmd.Args() - // if args[0] == "cluster" && args[1] == "getkeysinslot" { - // return args[2].(int) - // } + args := cmd.Args() + if args[0] == "cluster" && args[1] == "getkeysinslot" { + return args[2].(int) + } - // cmdInfo := c.cmdInfo(ctx, cmd.Name()) - return cmdSlot(cmd, cmdFirstKeyPos(cmd, nil)) + return cmdSlot(cmd, cmdFirstKeyPos(cmd)) } func cmdSlot(cmd Cmder, pos int) int { @@ -1797,6 +1795,7 @@ func cmdSlot(cmd Cmder, pos int) int { func (c *ClusterClient) cmdNode( ctx context.Context, + cmdName string, slot int, ) (*clusterNode, error) { state, err := c.state.Get(ctx) @@ -1805,7 +1804,10 @@ func (c *ClusterClient) cmdNode( } if c.opt.ReadOnly { - return c.slotReadOnlyNode(state, slot) + cmdInfo := c.cmdInfo(ctx, cmdName) + if cmdInfo != nil && cmdInfo.ReadOnly { + return c.slotReadOnlyNode(state, slot) + } } return state.slotMasterNode(slot) } diff --git a/ring.go b/ring.go index 367a542e9..032f1a52b 100644 --- a/ring.go +++ b/ring.go @@ -691,8 +691,7 @@ func (c *Ring) cmdInfo(ctx context.Context, name string) *CommandInfo { } func (c *Ring) cmdShard(ctx context.Context, cmd Cmder) (*ringShard, error) { - cmdInfo := c.cmdInfo(ctx, cmd.Name()) - pos := cmdFirstKeyPos(cmd, cmdInfo) + pos := cmdFirstKeyPos(cmd) if pos == 0 { return c.sharding.Random() } @@ -760,8 +759,7 @@ func (c *Ring) generalProcessPipeline( cmdsMap := make(map[string][]Cmder) for _, cmd := range cmds { - cmdInfo := c.cmdInfo(ctx, cmd.Name()) - hash := cmd.stringArg(cmdFirstKeyPos(cmd, cmdInfo)) + hash := cmd.stringArg(cmdFirstKeyPos(cmd)) if hash != "" { hash = c.sharding.Hash(hash) } From b53cf1ca8429d455d574751d05436df4bbeacfd4 Mon Sep 17 00:00:00 2001 From: ofekshenawa <ofek.shenawa@redis.com> Date: Wed, 29 Nov 2023 16:31:25 +0200 Subject: [PATCH 3/3] remove cmdInfo from ring --- ring.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ring.go b/ring.go index 032f1a52b..0c031b5a6 100644 --- a/ring.go +++ b/ring.go @@ -678,18 +678,6 @@ func (c *Ring) cmdsInfo(ctx context.Context) (map[string]*CommandInfo, error) { return nil, firstErr } -func (c *Ring) cmdInfo(ctx context.Context, name string) *CommandInfo { - cmdsInfo, err := c.cmdsInfoCache.Get(ctx) - if err != nil { - return nil - } - info := cmdsInfo[name] - if info == nil { - internal.Logger.Printf(ctx, "info for cmd=%s not found", name) - } - return info -} - func (c *Ring) cmdShard(ctx context.Context, cmd Cmder) (*ringShard, error) { pos := cmdFirstKeyPos(cmd) if pos == 0 {