Fixing a TODO in go-redis: Using CommandInfo to Route Commands

A few weeks ago I was reading through go-redis internals when I hit this comment sitting right above cmdFirstKeyPos:

The function below it had a hand-maintained map of approximately 40 "keyless" commands (commands with no key arguments like PING, INFO, CLIENT LIST etc.), a few hardcoded special cases, and then returned 1 for everything else. The problem? go-redis already fetches COMMAND INFO from Redis and caches the entire command metadata, including exactly which argument index holds the first key (FirstKeyPos). But routing was ignoring all of it.

So I opened issue #3803 and sent a PR. It got merged as #3804. Here's what actually happened.

Why This Mattered

Every time go-redis routes a command to the right cluster node or ring shard, it needs to know where the first key sits in the command's argument list.

• SET foo bar --> key is at index 1.
• ZADD myset 1 member --> key is at index 1.
• EVAL script 1 mykey --> key is at index 3 (if numkeys > 0).

The old approach:

This worked fine for core Redis commands. But it had two real problems:

1. The keyless map needed manual updates. Every time Redis ships a new keyless command, someone has to remember to add it. Miss one, and the client tries to read a key from an argument that isn't a key, routing the command to the wrong shard.
2. Redis module commands were invisible. A module like RediSearch or RedisTimeSeries can register commands with a FirstKeyPos of 0 (keyless) or something non-standard. The client had no idea that it just returned 1 and hoped for the best.

The fix was right there in the code: CommandInfo.FirstKeyPos. This is the exact same field Redis reports back when you run COMMAND INFO get in redis-cli. The client was already fetching it and caching it. It just wasn't being used for routing.

How the Cache Works

Before getting into the changes, it's worth understanding how the command info cache works in go-redis.

How the Cache Works

The cache uses sync.Once internally, so the COMMAND INFO fetch happens exactly once across the lifecycle of the client. After that, it's just a map read.

The Maintainer's Constraint

When I opened the issue, @ndyakov pointed out something important:

"keep in mind the hardcoded value helps with that it will determine this first key position without the need of a round trip"

This is the key constraint. The routing path --> the code that decides which cluster node to send your command to must not trigger a network call. Routing has to be synchronous and fast. If the cache isn't populated yet, fine, fall back to the hardcoded table. But don't initiate a COMMAND INFO fetch just to route a single command. This shaped everything that followed.

What Changed

1. cmdFirstKeyPosWithInfo --> the new resolution order

The old function became cmdFirstKeyPosWithInfo(cmd Cmder, info CommandInfo) int. The info parameter is nil when the cache is cold, and a real CommandInfo when it's warm. The resolution order inside looks like this,

What Changed I

The keyless map check stays before the cache lookup. @ndyakov's concern was valid. The map is an O(1) in-memory check with no dependency on cache state. It's the right fast-path for those known commands. The cache only gets consulted for commands not in the map, which is where the improvement actually lands: module-registered commands, or any future Redis command we haven't added to the table yet.

2. cmdsInfoCache.Peek() --> read without triggering a fetch

refreshLock was upgraded from sync.Mutex to sync.RWMutex. Get() and Refresh() both write, so they keep Lock(). Peek() only reads so it uses RLock(). Multiple goroutines routing commands in parallel can all call Peek() simultaneously without blocking each other.

3. ClusterClient.cmdInfoPeek --> thin cache helper

4. Minimizing lock acquisitions in pipeline loops

A pipeline can contain hundreds of commands. The original naive approach was calling Peek() inside the loop per command. So it would acquire and release an RLock for every single command. Not catastrophic, but wasteful.

slottedKeyedCommands in osscluster.go was the worst offender: it was calling cmdInfoPeek (one RLock) and then cmdSlot which called it again (another RLock). Two lock acquires per command just to look up the same name twice.

The fix: peek the full map once before the loop, do a plain map lookup inside,

5. Fixing a subtle inconsistency in multi-shard commands

executeMultiShard splits a command like MGET key1 key2 key3 across multiple slots. It computed firstKeyPos once, then called createSlotSpecificCommand which independently computed it again via another Peek(). Between these two calls, if the cache transitioned from cold to warm, they could get different answers. executeMultiShard extracts keys from one offset, createSlotSpecificCommand rebuilds args from a different offset, producing a malformed command.

Fix: compute firstKeyPos once in executeMultiShard and pass it all the way down as an argument.

6. Ring --> honest about what's not done yet

Ring has a cmdsInfoCache with a valid fetch function wired up, but Get() is never called in the ring routing path. Peek() would always return nil for Ring. The extra RLock per command was pure overhead. Rather than silently doing nothing useful, both ring call sites now explicitly pass nil with a TODO comment,

The proper fix for Ring would thread ctx into cmdShard and call cmdsInfoCache.Get lazily on first use, that touches method signatures across the ring routing path so it's a follow-up.

Impact

Before this change:

• Module-registered keyless commands (not in keylessCommands) were routed as if their key was at index 1. If that argument wasn't actually a key, the client hashed the wrong thing and sent the command to the wrong node, resulting in a MOVED redirect or silent misbehaviour.
• The keylessCommands map was a manual maintenance burden. Easy to forget when Redis adds something new.
• The dead case "publish": return 1 in the switch could never be reached (since "publish" was in keylessCommands and exited early), but was quietly there causing confusion.

After this change:

• Once the command info cache is warm (after the first COMMAND INFO fetch that happens naturally), routing uses Redis's own answer for any command not already covered by the hardcoded fast-paths.
• No extra round-trips. Ever. The whole point of Peek().
• The keylessCommands table stays as the cold-start fallback with same behaviour on startup as before.
• Pipeline routing acquires the cache lock once per pipeline, not once per command.
• firstKeyPos computed exactly once per multi-shard command dispatch, passed through rather than recomputed.

What I Learnt

Working through @ndyakov's review was genuinely useful. The first version I sent had the cache check before the keylessCommands map. @ndyakov pointed out the map should run first as it's a known set, cache-independent, and the right fast-path.

If you're new to open source and looking for a place to start, read the source of tools you already use. The TODO was sitting right there in plain sight. You don't need to invent something novel; sometimes the codebase is already telling you exactly what it needs.

The full diff is at redis/go-redis#3804.