Cluster pipeline reorders commands issued on the same key

[afshinsa]

I have a pipeline of Set followed by Expire, but they arrive out of order and therefore the key never expires. What seems to be happening is this

Set -> wrong master (get MOVED) Expire -> correct master Set -> correct master (therefore out of order)

Some logs

My application log (how the pipeline is generated)

2021-10-28T12:57:39.627-0700	DEBUG	producer/redisproducer.go:143	Mirror	{"num-cmds": 2}
2021-10-28T12:57:39.627-0700	DEBUG	producer/redisproducer.go:151	Mirror	{"cmd": "SET", "key": "000000000979"}
2021-10-28T12:57:39.627-0700	DEBUG	producer/redisproducer.go:151	Mirror	{"cmd": "EXPIRE", "key": "000000000979"}

Logs from go-redis (I added these):

_processPipeline Redis<127.0.0.1:7003 db:0> 1
_processPipeline Redis<127.0.0.1:7001 db:0> 1
writeCmds 1
writeCmds SET 000000000979 val
writeCmds 1
writeCmds EXPIRE 000000000979 60
_processPipeline Redis<127.0.0.1:7001 db:0> 1
writeCmds 1
writeCmds SET 000000000979 val: MOVED 4024 127.0.0.1:7001

How the commands appear out of order on the cluster master 7001. As you can see they're arriving on the same connection.

1635451059.627635 [0 127.0.0.1:62035] "EXPIRE" "000000000979" "60"
1635451059.636649 [0 127.0.0.1:62035] "SET" "000000000979" "val"

So, shouldn't the client guarantee that pipelined commands destined for the same master to be executed in order?

[afshinsa]

A bit more info: go-redis/v8 redis 6.0.15 everything runs on my laptop and the cluster topology is not changing while running the test.

[monkey92t]

I am very puzzled. In the example, why does the same key reach different nodes?

[afshinsa]

Ok, I found the problem in this function:

func (cmd *baseCmd) stringArg(pos int) string {
    if pos < 0 || pos >= len(cmd.args) {
        return ""
    }
    s, _ := cmd.args[pos].(string)
    return s
}

cmd.args is of type []interface{} and in my application the args of of type []uint8 so the type assertion here actually fails and s is always '' and therefore the returned slot is always a random number based on this code:

func Slot(key string) int {
    if key == "" {
        return RandomSlot()
    }
    key = Key(key)
    return int(crc16sum(key)) % slotNumber
}

So, we only get lucky when both random slots point to the same node.

It seems to me that stringArg is making a wrong assumption that the arguments are always of type string since baseCmd definition is []interface{}

[afshinsa]

It's probably ok for stringArg to assume the argument is string because I guess that's what the function is intended for. But then the caller has made the wrong assumption that the key is always a string.

[monkey92t]

Can you provide go code examples?

for example:

db := redis.NewClusterClient(...)
pipeline := db.Pipeline()
pipeline.Get(ctx, xxx)
pipeline.Set(ctx, xxx)

[afshinsa]

Something like this (this is not my actual code so if you see any unrelated bug/typo ignore it):

type command struct {
   name string
   args [][]byte
}

func produce(ctx context.Context, client *redis.ClusterClient, cmds []command) error {	
	_, err := client.Pipelined(ctx, func(pipe redis.Pipeliner) error {
		for _, cmd := range cmds {		
			pipeArgs := make([]interface{}, len(cmd.args)+1)
			pipeArgs[0] = cmd.name
			for i, arg := range cmd.args {
				pipeArgs[i+1] = arg
			}
			pipe.Do(ctx, pipeArgs...)
		}
		return nil
	})
     return err
}

I worked around the problem by casting the key to string in my code, but as far as go-redis goes, I think either the code needs to be fixed not to assume the key is always of type string or there should be a comment somewhere making this assumption clear.

[monkey92t]

You can try to use it like this:

func produce(ctx context.Context, client *redis.ClusterClient, cmds []redis.Cmd) error {	
	_, err := client.Pipelined(ctx, func(pipe redis.Pipeliner) error {
		for _, cmd := range cmds {		
			pipe.Process(ctx, cmd)
		}
		return nil
	})
     return err
}

produce(ctx, client, []redis.Cmd{
        redis.NewCmd(ctx, "set", "key", 1024),
        redis.NewCmd(ctx, "expire", "key", time.Hour),
})

[afshinsa]

Thanks, as I mentioned I worked around this issue by casting the key to string in my code, but that's irrelevant because the code is not honoring the API by making the assumption that an interface{} is always a string. So, as I said above either the code needs to be fixed or there should be a comment making it clear that the key has to be of type string although the API can take an interface{}

[github-actions[bot]]

This issue is marked stale. It will be closed in 30 days if it is not updated.