@duangg these commands have their own callback to find the argument indexes that refer to keys.
the syntax for these is complicated and is not possible to get the key arguments just by looking at fixed indexes.
As far as i know the changes you made aren't affecting the software at all.
Please correct me if i'm wrong, or give an example of a problem that it fixes.
Thanks.

@duangg these commands have their own callback to find the argument indexes that refer to keys.
the syntax for these is complicated and is not possible to get the key arguments just by looking at fixed indexes.
As far as i know the changes you made aren't affecting the software at all.
Please correct me if i'm wrong, or give an example of a problem that it fixes.
Thanks.

Recently I use go-redis to scan my redis-cluster for big keys and other infomations. go-redis
will cache infos return by COMMAND, use first key position to find key and calculate slot. Because the position for MEMORY is 0, go-redis will return a random slot

Is there any better solution for this problem? Should I only change info about MEMORY?

Thanks

@duangg i suppose the right thing to do is use COMMAND GETKEYS.
ideally go-redis should be able to tell by the COMMAND info which commands can be resolved by using the first,last,step, and which require a call to COMMAND GETKEYS each time.
I think adding such a tip and improving go-redis is the way to go since the fix you suggested may provide false result.

@oranagra Thanks for your advice, I will try to fix this bug in go-redis. You can close this PR

@duangg i think that in order to properly fix go-redis, we might have to add some hint in redis.
but that will cause backwards compatibility issues.
please try to figure it out (i haven't looked at go-redis code), and post here or open another PR with your conclusions.

@oranagra First, go-redis use COMMAND to get all redis command info
ring.go

func (c *Ring) cmdsInfo() (map[string]*CommandInfo, error) {
	shards := c.shards.List()
	var firstErr error
	for _, shard := range shards {
		cmdsInfo, err := shard.Client.Command(context.TODO()).Result()
		if err == nil {
			return cmdsInfo, nil
		}
		if firstErr == nil {
			firstErr = err
		}
	}
	if firstErr == nil {
		return nil, errRingShardsDown
	}
	return nil, firstErr
}

Then it try to find first key position from command info
command.go

func cmdFirstKeyPos(cmd Cmder, info *CommandInfo) int {
	switch cmd.Name() {
	case "eval", "evalsha":
		if cmd.stringArg(2) != "0" {
			return 3
		}

		return 0
	case "publish":
		return 1
	}
	if info == nil {
		return 0
	}
	return int(info.FirstKeyPos)
}

Then use first key postion to calculate key slot
cluster.go

func cmdSlot(cmd Cmder, pos int) int {
	if pos == 0 {
		return hashtag.RandomSlot()
	}
	firstKey := cmd.stringArg(pos)
	return hashtag.Slot(firstKey)
}

Because first key position in MEMORY is 0, it will return a random slot. The command will send to a random shard. I have run my program on different version of redis-cluster. In version 4.0.2, redis-server will return nil when it's in a wrong server, go-redis will throw an error. In version 6.0.5, redis-server will return with a MOVED response, go-redis can handle this situation properly, but I think it's unnecessary if go-redis have correct command info

Again, I think this approach is not enough.
You may argue that since go-redis only cares about the first key (and not others), and since it doesn't care about the key name itself, and it's not a problem to send the command to a random shard, then this PR is enough. I.e. If someone runs another sub-command of MEMORY, it doesn't any key.
Until some day we'll add a new sub-command of MEMORY that uses a key, and not from index 2.

However, what would be the correct "fix" for STRALGO?
You can't really know in advance the index of the key name.

I think we need to change the output of COMMAND to give an indication of which command has a getKeys callback and which one doesn't. Then go-redis has to understand that for these commands it has to call COMMAND GETKEYS each time.
the alternatives are either relying on the MOVED response, or implement more logic per command to know the arguments of each (like it does for EVAL)

ohh, i now see that there already is a movablekeys flag.
so that's the indication that the first/last/step fields should not be trusted, and a GETKEYS command must be executed on every call.

it's even documented: https://redis.io/commands/command

@oranagra Thanks for your explanation, I understand why I shouldn't change the configuration.

I have reed the document about command and memory, but not familiar enough, my considerations aren't comprehensive enough.

Your advice to implement more logic maybe better for this situation.

Thanks for your help

@duangg FYI, i see go-redis has a fix now: redis/go-redis#1400

@oranagra Thanks, it should resolve my problem