fix: retry transport failures once and set TCP keepalive/nodelay

Summary

A Valkey connection that dies while a command is in flight raised the error straight to the caller. On the cache path that becomes a failed request — a silent miss with IGNORE_EXCEPTIONS, a 500 without it — even though the failure usually says nothing about the command that hit it. ConnectionManager hands out a pooled connection whose peer may have gone away while it sat idle (server restart, failover, a load balancer reaping an idle socket), so the first write fails on a connection that was already dead.

This retries such a command once, configurable via VCACHE_COMMAND_RETRIES (0 restores the previous behavior). Worth noting what does not need it: a disconnect that happens between commands is already absorbed, because redis-rs reconnects in the background and the next command transparently waits for the new connection. The retry only matters for a connection that dies with a command already in flight.

Only operations whose repeated application is indistinguishable from a single application are retried — reads, SET, DEL, EXPIRE, PERSIST, SCAN, FLUSHDB, SCRIPT LOAD. These are excluded, because a second application would be observable:

Not retried Why
INCRBY / DECRBY double-counts if the first reply was lost
SET NX (add) the boolean is the result; a retry reports False after a successful set
lock acquire / release / extend same — the boolean is the mutual-exclusion answer
EVAL / EVALSHA arbitrary script side effects
arbitrary pipelines may have partially applied
LPUSH / RPUSH / RPOP / LREM / LTRIM queue semantics; a retry duplicates or over-removes
BLMOVE / BLMPOP a retry can orphan an element into the processing list

For comparison, redis-py 8 retries every command including the blocking ones, ten times by default; valkey-py retries nothing unless you opt in. Neither has a carve-out for blocking commands. This sits between them deliberately: a blocking consumer already has its own retry loop, and silently re-issuing a BLMOVE trades a recoverable error for a stranded queue element.

Second, unrelated-looking but same root area: TCP options

redis-rs defaults TcpSettings to no keepalive and nodelay: false, and vcache never overrode it. Two consequences:

  • Nagle was on. Commands are multiplexed with a 1000-deep pipeline buffer, so a small write frequently happens while earlier bytes are still unacked — exactly where Nagle waits for the ACK and delayed-ACK can add up to ~40ms. Both redis-py and valkey-py set TCP_NODELAY unconditionally.
  • No keepalive, so a silent peer death was never detected. If a peer goes away without sending a FIN (powered-off node, a firewall dropping the flow, NAT eviction), nothing arrives — and BLMOVE deliberately runs with no response timeout, so it waited forever and the consumer wedged with no error to log or retry. SO_KEEPALIVE alone would not help either; Linux's default idle time is 7200s.

Now set on all three client-construction paths (standard, sentinel master, cluster): keepalive 30s idle / 5s interval / 3 probes, TCP_USER_TIMEOUT 45s, and TCP_NODELAY. Timings match redis-py's defaults, giving ~45s to detect a dead peer. This is what closes the wedged-consumer gap — the retry cannot, since there is no error to retry on.

Testing

Full suite: 107/107 passing, run twice — once with the new default and once with VCACHE_COMMAND_RETRIES=0 — against a real Valkey on Python 3.14.5. Also cargo test, cargo clippy and cargo fmt clean for the new code.

Behavior verified by A/B with a real server-side disconnect, via the new examples/kill_retry.rs harness plus an external valkey-cli CLIENT KILL TYPE normal during the run:

kills failed ops
MODE=get VCACHE_COMMAND_RETRIES=0 2 2 (broken pipe) / 25,527
MODE=get VCACHE_COMMAND_RETRIES=1 2 0 / 29,029
MODE=blmove VCACHE_COMMAND_RETRIES=1 2 2 — unchanged, by design

Socket options confirmed applied with ss -tnieo, which shows timer:(keepalive,25sec,0) on our sockets counting down from 30s (no keepalive timer at all before this change).

Note that two earlier attempts at the repro were misleading and are worth avoiding: CLIENT KILL is not callable from a Lua script, and killing the connection between commands shows no failure even with retries disabled, because the background reconnect already covers that case.

examples/kill_retry.rs is a manual harness — it needs an external kill, so it proves nothing on its own in CI. Happy to drop it if you would rather not carry another compiled target; it is included because getting the reproduction right was not obvious.

AI disclosure: Claude Code (Opus 5) — investigation, patch, docs and the test harness drafted with AI; redis-py/valkey-py comparison read from their installed sources rather than from memory; all test runs and the A/B above executed and reviewed by me.

Merge request reports

Loading
Loading