chore(deps): replace stale dependency github.com/go-redis/redis/v8#7010
chore(deps): replace stale dependency github.com/go-redis/redis/v8#7010stoewer wants to merge 3 commits intografana:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR migrates the project from github.com/go-redis/redis/v8 to the renamed module github.com/redis/go-redis/v9, updates option field names accordingly, and adjusts MSet behavior in Redis Cluster mode to avoid CROSSSLOT transaction errors.
Changes:
- Switch imports from
go-redis/redis/v8toredis/go-redis/v9. - Update
UniversalOptionsfield names (IdleTimeout→ConnMaxIdleTime,MaxConnAge→ConnMaxLifetime). - In
MSet, use a non-transactional pipeline when the client is aClusterClientto avoid cross-slot transaction failures.
Reviewed changes
Copilot reviewed 4 out of 145 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cache/redis_client.go | Updates redis client options for v9 and changes MSet pipelining behavior for cluster clients. |
| pkg/cache/redis_cache.go | Updates go-redis import path to v9. |
| pkg/cache/redis_cache_test.go | Updates go-redis import path to v9 for tests. |
| go.mod | Replaces v8 dependency with github.com/redis/go-redis/v9. |
| // In cluster mode, cross-slot transactions are not supported. Use a non-transactional | ||
| // pipeline to allow keys on different slots, consistent with how MGet handles clusters. | ||
| _, isCluster := c.rdb.(*redis.ClusterClient) | ||
| var pipe redis.Pipeliner | ||
| if isCluster { | ||
| pipe = c.rdb.Pipeline() | ||
| } else { | ||
| pipe = c.rdb.TxPipeline() | ||
| } |
There was a problem hiding this comment.
HIGH: This changes MSet semantics in cluster mode from (per-node) transactional to non-transactional, which can lead to partial writes if some SETs fail. If callers rely on MSet being all-or-nothing, consider either (a) documenting explicitly that cluster-mode MSet is best-effort/non-atomic, or (b) preserving atomicity when possible (e.g., if all keys map to the same hash slot, use a transaction; otherwise fall back to non-transactional pipeline).
| // In cluster mode, cross-slot transactions are not supported. Use a non-transactional | ||
| // pipeline to allow keys on different slots, consistent with how MGet handles clusters. | ||
| _, isCluster := c.rdb.(*redis.ClusterClient) | ||
| var pipe redis.Pipeliner | ||
| if isCluster { | ||
| pipe = c.rdb.Pipeline() | ||
| } else { | ||
| pipe = c.rdb.TxPipeline() | ||
| } |
There was a problem hiding this comment.
MEDIUM: The new cluster-specific behavior in MSet isn’t covered by tests. Since this is a behavioral change (avoiding CROSSSLOT by removing transactions in cluster mode), add a test that exercises the cluster/non-cluster branching. If it’s hard to stand up a real Redis Cluster in unit tests, consider extracting pipeline selection into a small helper and unit-testing that helper with a minimal stub.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
stoewer
force-pushed
the
replace-stale-dep-go-redis
branch
from
315d30e to
8c748f9
Compare
What this PR does:
Upgrades
github.com/go-redis/redis/v8togithub.tiyicn.workers.dev/redis/go-redis/v9(the module path changed when the org was renamed fromgo-redistoredis). The two renamedUniversalOptionsfields (IdleTimeout→ConnMaxIdleTime,MaxConnAge→ConnMaxLifetime) are updated accordingly. In cluster mode,MSetnow uses a non-transactional pipeline to avoid CROSSSLOT slot constraint errors, consistent with the existing approach inMGet.Which issue(s) this PR fixes:
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]