If the first command in a pipeline fails with an non-nil error, all the results are replaced with that error

[sgp]

Expected Behavior

I am using the ring client and Pipelined to implement MGET, since it MGET doesn't work across shards:

	responses, pipeErr := iter.rdb.Pipelined(iter.ctx, func(p redis.Pipeline) error {
		for _, jobKey := range iter.jobKeys {
			_, _ = p.Get(iter.ctx, jobKey)
		}
		return nil
	})
	if pipeErr != nil && !errors.Is(pipeErr, redislib.Nil) {
		return nil, pipeErr
	}

	iter.jobs = iter.jobs[:0]
	for i, response := range responses {
		jobString, err := response.(*redislib.StringCmd).Result()
		if errors.Is(err, redislib.Nil) {
			continue // skip any job not found
		}

		iter.jobs = append(iter.jobs, job)
	}
	return iter.jobs, nil
}

I have a test that creates a bunch of keys, deletes all but the first and last one, and then fetches them using the pipelined get.

Prior to the upgrade to 9.14.0, I would always correctly get the first and last item in iter.jobs.

Current Behavior

Now, I always get the first, but sometimes not the last item.

Possible Solution

Reverting the change in 78be750 fixes the issue.

However, I don't think that is the fix. I think what's happening here is that the ring client's generalProcessPipeline is taking the first error in the pipeline and surfacing that as the entire error (see

go-redis/ring.go

Line 870 in 7405cff

return cmdsFirstErr(cmds)

).

Steps to Reproduce

Using a ring client with a few shards:

1. Create 100 items in Redis with keys named (key0, key1, ... key99). Put all the key names in a slice.
2. Delete every key except for key0 and key99.
3. Using Pipelined, issue GET commands for all keys, including the ones that were deleted. Make sure key0 is the first key in the pipeline and key99 is the last.
4. Check the results from the pipeline. If working properly, the error response for all keys except for key0 and key99 should be redis.Nil, while key0 and key.99 should be nil

Note: I suspect that this problem may be non-deterministic due to sharding, as well, as I've had this test case sometimes work, but often fail.

[htemelski-oss]

Hello @sgp, thanks for raising the issue.

[LINKIWI]

Chiming in to report that we are observing the same correctness regression in v9.14.0, surfaced by a test suite failure. Here is another minimal reproduction case:

package main

import (
        "context"
        "fmt"

        "github.com/redis/go-redis/v9"
)

func main() {
        ctx := context.Background()
        rdb := redis.NewClient(&redis.Options{})

        pipe := rdb.Pipeline()
        pipe.Do(ctx, "GET", "foo")
        pipe.Do(ctx, "SET", "foo", "bar")
        pipe.Do(ctx, "GET", "foo")
        res, err := pipe.Exec(ctx)

        fmt.Println(res, err)
}

1. Launch an empty Redis server
2. Run the above script

Expected behavior: return slice should be nil, OK, bar.
Actual behavior: return slice is nil, nil, nil.

[sgp]

@LINKIWI I guess this is not limited to the RingClient, so I'll amend the title of the issue lest someone think it's isolated to that area of the code.

[sgp]

@ndyakov I guess this is a duplicate issue of #3524 which appears to be fixed in #3525.

[mfridman]

Yep, noticed a similar thing when using a plain *redis.Client with .Pipeline() calls. If the first error is redis: nil then all subsequent errors are redis: nil even when there is data.

[FredrikL]

I also face this, since it has been fixed could we get an release version with this fix too ?

[ndyakov]

Please review https://github.com/redis/go-redis/releases/tag/v9.14.1 which should include the fix :
https://github.com/redis/go-redis/blob/v9.14.1/redis.go#L634