redis / go-redis

Redis Go client
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
20.2k stars 2.38k forks source link

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

Closed tasszz2k closed 2 months ago

tasszz2k commented 2 months ago

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

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

Go-Redis V9

https://github.com/redis/go-redis/blob/1da50b33ef44c54143b194edcba664d7503b8fc9/command.go#L78-L99

Go-Redis V8

https://github.com/redis/go-redis/blob/cae67723092cac2cb441bc87044ab9edacb2484d/command.go#L62-L87