Repository navigation
redis: make RealTime tail anchoring resilient to errors - #48
Merged
Merged
Conversation
The tail anchoring introduced in #47 cleared `first_read` before issuing the XREVRANGE calls and propagated their errors raw, with two consequences: * A transient error (Io, TryAgain) at the very first read is retried by the node loop, but with `first_read` already false the anchoring was skipped on retry, silently reverting un-anchored shards to `$` -- the very starvation bug #47 fixed. * In cluster mode, all shards start on the first node and rely on MOVED errors from the first read to be redistributed. XREVRANGE now hits the MOVED before the XREAD ever runs, and the raw error killed the node, so a RealTime + Latest consumer died at startup whenever its keys did not all live on the first node. Anchoring now applies the same error policy as the XREAD reply: MOVED triggers move_shards, Io reconnects, cluster errors are retried, and anything fatal is forwarded to the consumer via send_error instead of a generic ConsumerDied. `first_read` is only cleared once every shard is anchored; a retry resumes where it left off, as anchored shards are skipped via `id.is_some()`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #47, fixing two problems in the error paths of the per-shard tail anchoring.
Problems
1. Cluster mode broke at startup.
Cluster::runassigns all shards to the first node and relies on MOVED errors from the first read to redistribute them (move_shards). The anchoringXREVRANGEruns before the firstXREADand its errors were propagated raw via?, so a MOVED reply becameRedisErr::Moved, which the node loop treats as fatal. ARealTime+Latest(the default) consumer against a Redis cluster died withConsumerDiedwhenever its keys did not all live on the first connected node — the common case with several streams.Ask/ClusterDown/MasterDownwere similarly fatal here despite being retried on theXREADpath.2. A transient error at first read silently re-introduced the starvation bug.
first_readwas cleared before the fallible anchoring loop. If anXREVRANGEfailed with an error the node loop retries (Io → reconnect, TryAgain → sleep), the retry skipped the anchoring block entirely, and every still-unanchored shard fell back to sending$on eachXREAD— exactly the quiet-stream message loss #47 fixed, for the consumer's whole lifetime, with nothing surfaced.Fix
XREADreply: MOVED →move_shards(cluster reassigns; the destination node re-anchors sinceAddShardre-arms itsfirst_read), Io → reconnect, cluster errors → retry, anything else →send_errorso the consumer sees the actual error instead of a genericConsumerDied.first_readis cleared only after every shard is anchored. Error paths return with it still true, and a retry resumes where it left off — already-anchored shards are skipped viaid.is_some(), so the loop is idempotent.XREVRANGEis also routed throughsend_errorfor consistency.Tests
New unit tests for
parse_stream_id(valid/edge/invalid ids), and five new scenarios inrealtime-multi.rson top of the three from #47:quiet_history_not_starved_nor_replayed— a quiet stream with pre-existing history anchors at its own tail: no replay, and the message landing in the starvation window is not lost.absent_streams_all_delivered— streams that don't exist yet anchor at0-0; the very first entries are delivered.multiple_rounds— the anchor keeps advancing after first delivery across repeated starvation cycles.earliest_gets_everything—Earliestis unaffected by anchoring: full history plus starvation-window messages.wrong_type_key_errors— a stream key occupied by a plain string (WRONGTYPE onXREVRANGE) surfaces the actual backend error to the consumer. Verified this fails onmain(genericConsumerDied) and passes with the fix. Requires therediscrate's blocking client as a dev-dependency for setup.All pass locally on both
runtime-tokioandruntime-smolagainst Redis 8. The MOVED and Io-retry paths aren't covered by CI — they need a Redis cluster / fault injection; verified by code inspection against theXREADpath, which handles the identical errors.🤖 Generated with Claude Code