Skip to content

refactor(streams): keep each run in one Redis key - #99

Open
jruaux wants to merge 12 commits into
feature/resumable-streamsfrom
claude/streams-simplified-write-path
Open

jruaux wants to merge 12 commits into
feature/resumable-streamsfrom
claude/streams-simplified-write-path

Conversation

@jruaux

@jruaux jruaux commented Oct 29, 2026 •

Copy link
Copy Markdown

Problem

On feature/resumable-streams, every run is three keys (stream, meta hash, the scope's active slot), and every append() is a MULTI of 8 commands. Most of that is bookkeeping: each append doubles as the producer heartbeat, refreshing four TTLs and reading a cancel flag. The design also has two bugs:

  1. A live producer's stream expires during a long idle wait. keepalive() refreshes the heartbeat but not the stream or meta TTLs, so a tool call longer than stream_active_ttl makes the next append fail. fix(streams): retain active history during heartbeats #98 fixes this inside the current design.
  2. A producer keeps writing after a reader ended its run. A hand-driven producer quiet for longer than stream_alive_ttl is ended as producer_lost, but its next append() and finish() silently succeed.

What changes

One key per run: a Redis Stream, <prefix>:stream:<scope>:<id>. Everything about a run is an entry in it, and readers stop at the final entry.

Operation Commands
open() MULTI XADD s * t stream.start · XCFGSET s IDMP-DURATION 86400 · EXPIRE s <stream_max_run> NX EXEC, one round trip. created is the EXPIRE NX reply: only a stream with no TTL yet, one this call just created, gets one. Setting the IDMP window inside the transaction means nothing can write a pair that it then clears; a retried start sends the same value, which clears nothing
append() XADD s NOMKSTREAM IDMP <writer> <seq> * …: one command, no reads, no TTL writes. Appends from one writer go out one at a time, so a retry is never overtaken by enough others to push its pair out of Redis's 100-pair IDMP history. A writer can append at most stream_maxlen events
finish(), cancel(), a reader's producer_lost MULTI XADD s NOMKSTREAM IDMP end final … f 1 · EXPIRE s <stream_done_ttl> LT EXEC. The shared IDMP pair keeps one final entry however they race; LT means ending only ever shortens the run's lifetime
keepalive() (run by spawn()) A hidden stream.ping, only after half stream_silence_timeout without a write
Reader Checks before the first byte: 404 missing, 204 at the final entry, 409 cursor ahead, 410 trimmed history. Then XREAD BLOCK. Past stream_silence_timeout with nothing new, it writes stream.end {producer_lost}

Removed:

  • the meta hash, the scope's active slot, active(), exclusive and StreamBusy;
  • every WATCH transaction;
  • the per-stream heartbeat and the alive field;
  • the settings stream_active_ttl, stream_alive_ttl and stream_gone_ttl.

Added: the setting stream_silence_timeout, and stream_max_run as the run's hard lifetime (at most 86,400 s). Nothing extends a run: extend() is gone, because it could lengthen a run another process had already ended.

No trimming: no write uses MAXLEN. A producer that missed a cancel could otherwise trim the final entry away by writing past it. stream_maxlen now caps each writer's events, and append() raises past it.

The example app (examples/incident_triage_app) keeps its own per-user current-run pointer, for "one run per engineer" and "the phone finds the run". It claims the pointer with SET … NX, frees it with DELEX … IFEQ, and takes it over once its run has ended.

Behaviour:

  • Bugs 1 and 2 above can no longer happen. If this lands, it supersedes fix(streams): retain active history during heartbeats #98.
  • A cancel in the producer's own process stops it at once. A cancel arriving at another process ends the run for readers at once, but the producer isn't told and runs to completion. That's the trade-off Glance's design accepts, and it means a stop button doesn't stop model spend when the request lands on another worker.
  • Crash detection is a silence timeout (60 s by default), so a quiet producer must run inside keepalive(); spawn() does that.
  • An expired run is a 404, the same as an unknown ID. There's no 410 for it.
  • cancel() returns nothing, and finish() after someone else ended the run returns that final entry's ID.
  • A retried open() whose first reply was lost reports created = False; the run then ends as producer_lost after the silence timeout.

Size: stream_backend.py goes from 1,112 code lines on feature/resumable-streams to 815, excluding comments and docstrings.

Validation

Run locally with redis-py 8.0.1 on Redis 8.8, standalone and a 3-node OSS cluster:

  • The whole repo suite, with Redis on 6379 as in CI: 1,377 passed, 0 failed, 27 skipped (the Enterprise runs without an endpoint). The unit suite passes with no Redis reachable and meets the 80% coverage gate (84.4%).
  • test_stream_topologies.py checks the command count of each operation through INFO commandstats: open() is one XADD, one EXPIRE and, only when it creates the run, one XCFGSET; append() is one XADD. It also covers lost producers, keepalive(), cancels from the producer's own process and from another, and a cancel racing a finish() (held at EXEC with gated_exec), on both topologies.
  • The cluster client-compatibility test passes all 8 variants.
  • ruff, mypy and bandit are clean.
  • Every stream test that makes a Redis call uses a real server; the remaining unit tests make no Redis call.
  • Review: two independent read-only review rounds.
    • Round 1 reported 11 findings: 7 fixed, each with a test. The fixes: sequence numbers reserved before sending, so concurrent or retried appends are never silently dropped; EXPIRE … LT on ends; the IDMP window widened to the run's life; a Redis outage no longer counted as producer silence; earlier pings; and three fixes in the example.
    • Round 2 reported 6 findings: 3 fixed, with the test corrections it asked for. The fixes: a claim can't be taken over before its run opens; the example's cleanup no longer masks the original error. One more fix reduced false stream.gap events.
    • An external review then reported 6 more: writes after a cancel could trim the final entry; a held retry could duplicate an event; a cancel between open() and its XCFGSET lost its IDMP pair; extend() could lengthen a finished run; the example missed ends behind 20 later entries; and the 7.4 test failure. All 6 are fixed, each with a test that fails on the previous code and passes now.
    • A third, focused review of those fixes found the example could treat every live run as over when stream_done_ttl isn't shorter than stream_max_run: the example now refuses that at startup, and the SDK accepts any stream_done_ttl (a finished run keeps what lifetime it has left). It also found three tests that didn't prove their names (fixed).
    • The rest are accepted trade-offs, listed under Limitations.

Limitations

  • Not tested on Redis Enterprise or Cloud, Sentinel, other redis-py versions, or Redis 8.6.2.
  • A creating open() whose reply is lost reports created = False: no producer starts, and the run ends as producer_lost after the silence timeout.
  • A start retried after the run expired creates it again, because nothing remembers it existed. created means "no live stream with this ID", not "first time ever".
  • stream_done_ttl is a maximum: a run that ends with less of its lifetime left keeps that shorter time.
  • Same-value XCFGSET keeping IDMP history was verified on Redis 8.8 only. This PR adds Redis 8.6.2, the supported minimum, to the CI integration matrix, so the stream tests check it there; I couldn't confirm that the redis:8.6.2 image tag exists.
  • The final write waits behind appends still in flight: an append left running in a task of its own holds up the end, as spawn()'s docstring now says.

🤖 Generated with Claude Code

jruaux and others added 3 commits October 29, 2026 13:10
…d a process lease

An append is now XADD plus one HMGET of status and cancel in a plain pipeline: 2 server commands instead of 8 in MULTI. open() writes a stream.start entry and gives every key a stream_max_run TTL, so nothing slides and a live producer's keys never lapse during an idle wait. An append past half the lifetime extends it.

The per-stream heartbeat, keepalive(), attach(), writer fencing, StreamFenced and the started and alive meta fields are gone. Each process keeps one lease: a set of the runs it still has a writer for, rewritten every stream_lease_interval, plus a cancel set other processes write to. Readers end a running run missing from its owner's lease as producer_lost; open() takes over a slot held by such a run. suspend() and resume() hand a run to another process; a resume of a run its owner still vouches for raises StreamBusy.

append() and finish() raise StreamFinished when a reader already ended the run, cancel() ends a suspended run, readers stop on a run whose entries expired, and each lifespan gets its own process ID so forked workers hold separate leases.

Docs still describe the old API.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On redis-py 8.0.1, an async RedisCluster whose transactional pipeline was executed outside async with can fail to close afterwards with 'Event loop is closed'. Every pipeline the backend runs now releases its connection on exit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the stream_case fixture and gated_exec from #98. stream_case runs a test against standalone Redis, and against an OSS Cluster or Redis Enterprise when STREAMS_TEST_CLUSTER_URL or STREAMS_TEST_ENTERPRISE_URL is set, with its own key prefix and cleanup.

test_stream_topologies covers appends that write no TTLs, a crashed process reported lost and its scope freed, a dropped writer reported lost, a cancel reaching another process during a tool call, suspend and a once-only resume, cancelling a suspended run, a reader whose end aborts when the run is resumed before its EXEC, and concurrent starts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.07317% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.35%. Comparing base (356dd7a) to head (99b27df).

Files with missing lines Patch % Lines
src/redis_fastapi/stream_backend.py 91.82% 13 Missing ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           feature/resumable-streams      #99      +/-   ##
=============================================================
+ Coverage                      96.21%   96.35%   +0.14%     
=============================================================
  Files                             17       17              
  Lines                           3748     3536     -212     
=============================================================
- Hits                            3606     3407     -199     
+ Misses                           142      129      -13     
Flag Coverage Δ
integration 80.17% <90.24%> (+0.77%) ⬆️
unit 84.38% <32.31%> (-2.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jruaux and others added 2 commits October 29, 2026 13:36
One _retry() helper replaces the three retry loops in _pipeline, _guarded and _create. _final_xadd() builds the final entry for finish, a cancel of a suspended run and a reader ending a lost run. _vouching() holds a run in the lease while open() or resume() decides, and _check_live() is the one guard for append, extend and suspend.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the standalone-only copies of eight scenarios that test_stream_topologies runs on every topology, after moving their extra assertions there: suspended status, TTL and sequence numbers, a second cancel, and no stray meta after concurrent starts. collect(), crash() and configure() move to tests/stream_support.py, replacing per-file copies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jruaux
jruaux marked this pull request as ready for review October 29, 2026 12:43
jruaux and others added 2 commits October 29, 2026 15:30
Remove the hand-written client and pipeline fakes and the MagicMock/AsyncMock clients from the stream unit tests. What they checked now runs on real Redis, on every topology stream_case is given: the support probe on RESP2 and RESP3 and against an unreachable server, detection caching, the commands one append runs (read from INFO commandstats: one XADD, one HMGET), lease renewal and cancel collection, a dropped writer leaving the lease, every producer state a quiet reader can find, and resume() and reads through RESP2 and RESP3 clients.

The unit tests left make no Redis call; a backend there gets a client factory that fails the test if called.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A run is now one Redis Stream, <prefix>:stream:<scope>:<id>, and nothing else: the meta hash, the scope's active slot, the process lease and the cancel sets are gone, and so are every WATCH transaction and the per-stream heartbeat.

open() writes the start entry and sets the run's lifetime in one MULTI; created is the reply of EXPIRE ... NX, and a creating open widens the stream's IDMP window to the run's life. append() is one XADD. finish(), cancel() and a reader's producer_lost end share one IDMP pair, so a run has one final entry, and EXPIRE ... LT lets ending only shorten it. A reader that sees nothing for stream_silence_timeout ends the run; keepalive(), which spawn() runs, pings a quiet producer.

Removed: active(), exclusive, StreamBusy, suspend(), resume(), and the settings stream_gone_ttl, stream_lease_interval and stream_lease_ttl. Added: stream_silence_timeout. A cancel from another process ends the run for readers but no longer reaches its producer. The incident triage example keeps its own current-run pointer per engineer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jruaux jruaux changed the title refactor(streams): simplify the write path with creation-time TTLs and a process lease refactor(streams): keep each run in one Redis key Oct 29, 2026
jruaux and others added 5 commits October 29, 2026 17:54
The test app no longer has an active route, so TestUnsupportedServer hit a 404 instead of the 503 it asserts; it runs only against a server without stream support, the redis 7.4 CI job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No write trims a run any more: XADD MAXLEN could remove the final entry once a producer that missed a cancel wrote past it. stream_maxlen now caps each writer's events instead, and append() raises past it.

Appends from one writer go out one at a time, so a retry is never overtaken by enough appends to push its IDMP pair out of Redis's 100-pair history.

open() sets the stream's IDMP window (a constant day) inside its creating transaction, so no cancel can land before it and lose its pair; a retried start sends the same value, which clears nothing. extend() is gone: it could lengthen a run another process had already ended. stream_done_ttl must be shorter than stream_max_run.

The incident triage example tells an ended run by its TTL rather than its last 20 entries, and uses the renamed settings; its claim has tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8.6.2 is the oldest server resumable streams support, and the IDMP behaviour the stream tests rely on was only checked on 8.8.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The guide, the API reference and the configuration page still described the meta hash, the scope's active slot, the producer heartbeat, active(), exclusive, StreamBusy, attach(), StreamFenced, extend(), trimming by stream_maxlen and the removed settings.

They now describe the run as one stream, the fixed run lifetime, the per-writer event cap, silence detection with keepalive(), a cancel that does not reach a producer in another process, and how an application keeps its own current-run pointer for one run at a time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Requiring stream_done_ttl shorter than stream_max_run only served the incident triage example, which tells an ended run by its TTL; it rejected settings that work, such as REDIS_STREAM_DONE_TTL=3600 with the default lifetime. Ending only ever shortens a run, so a longer stream_done_ttl means a finished run keeps what lifetime it has left.

The example now checks the assumption itself at startup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants