redis/go-redis

Incorrect Key Position Returned for Commands When Command Info is Unavailable in Go-Redis v8

tasszz2k opened this issue · 0 comments

Expected Behavior

When the command info cannot be retrieved, Go-Redis should return the correct key position for the command, or an appropriate error. The expected behavior is for the library to handle the ZADD command properly without requiring a preceding PING command.

Current Behavior

In Go-Redis v8, if command info cannot be retrieved, the library defaults to returning an index of 0 (which is the command name, in this case, ZADD). This behavior is incorrect. However, Go-Redis v9 has fixed this issue.

Additionally, when used with redis-coordinator (custom Redis-Proxy), the client fails to retrieve command info. The issue is resolved if a PING command is issued first, which forces the client to call the Redis server and retrieve the info correctly. This explains why adding a PING command before executing ZADD in Go-Redis v8 avoids the error.

Possible Solution

  • The fix for this issue has already been implemented in Go-Redis v9. One possible solution is to upgrade to v9, which return index 1 instead of index 0.
  • Another workaround for Go-Redis v8 users is to ensure that the PING command is called before operations like ZADD to avoid the error.

Steps to Reproduce

  1. Set up a Redis cluster using a redis-coordinator that does not support the COMMAND command.
  2. Initialize a Redis cluster client using Go-Redis v8.
  3. Perform a ZADD operation without calling PING first.
  4. Observe the incorrect behavior where ZADD fails due to the wrong index being returned.
package main

import (
	"context"
	"fmt"
	"time"

	redisv8 "github.com/go-redis/redis/v8"
)

var ctx = context.Background()

func main() {
	rdb := redisv8.NewClusterClient(
		&redisv8.ClusterOptions{
			Addrs: []string{
				"10.50.69.55:9736",
			},
			Password:    "xxxxxxxxx",
			DialTimeout: time.Second * 5,
			IdleTimeout: time.Second * 6,
		},
	)
	defer rdb.Close()

	zaddErr := rdb.ZAdd(
		ctx, "mySortedSet11", &redisv8.Z{
			Score:  1.0,
			Member: "member1",
		},
	).Err()

	if zaddErr != nil {
		fmt.Println("ZADD command failed:", zaddErr)
	}
}
  1. Now, add the PING command before the ZADD and see that the issue is resolved.

Context (Environment)

This issue affects environments using Redis with redis-coordinator in Go-Redis v8. In this setup, the lack of support for the COMMAND command in the coordinator causes problems when the client fails to retrieve the command info, resulting in incorrect behavior.

By issuing a PING command first, the client calls the Redis server and retrieves the necessary info, avoiding the error.

Detailed Description

Go-Redis v8 defaults to returning index 0 when command info is not available, which is incorrect for some Redis commands like ZADD. In scenarios where redis-coordinator is used and doesn't support the COMMAND command, the client fails to get command info unless a PING is sent first, which mitigates the issue.

This bug has been fixed in Go-Redis v9, where the correct index is returned even when command info is not available.

Possible Implementation

  • Encourage users to upgrade to Go-Redis v9, which has a proper fix.
  • For Go-Redis v8, document that users should issue a PING before commands like ZADD to avoid the issue, or explore a backport of the v9 fix if necessary.

Go-Redis V9

go-redis/command.go

Lines 78 to 99 in 1da50b3

func cmdFirstKeyPos(cmd Cmder) int {
if pos := cmd.firstKeyPos(); pos != 0 {
return int(pos)
}
switch cmd.Name() {
case "eval", "evalsha", "eval_ro", "evalsha_ro":
if cmd.stringArg(2) != "0" {
return 3
}
return 0
case "publish":
return 1
case "memory":
// https://github.com/redis/redis/issues/7493
if cmd.stringArg(1) == "usage" {
return 2
}
}
return 1
}

Go-Redis V8

go-redis/command.go

Lines 62 to 87 in cae6772

func cmdFirstKeyPos(cmd Cmder, info *CommandInfo) int {
if pos := cmd.firstKeyPos(); pos != 0 {
return int(pos)
}
switch cmd.Name() {
case "eval", "evalsha":
if cmd.stringArg(2) != "0" {
return 3
}
return 0
case "publish":
return 1
case "memory":
// https://github.com/redis/redis/issues/7493
if cmd.stringArg(1) == "usage" {
return 2
}
}
if info != nil {
return int(info.FirstKeyPos)
}
return 0
}