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_NODELAYunconditionally. - 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
BLMOVEdeliberately runs with no response timeout, so it waited forever and the consumer wedged with no error to log or retry.SO_KEEPALIVEalone 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.