fix(http): stream Brotli room exports incrementally - #865
Conversation
|
Checked at
One suggestion: This is the fix I offered on #861, so I won't open a competing PR. |
efc08de to
a0616b8
Compare
|
Updated the regression on current main: the ASGI harness now requires the first compressed body message after exactly one source chunk, so buffering three chunks still fails. Local full checks pass (783 passed, 1 skipped); pushed as f3d7547. |
WIZARDspace
left a comment
There was a problem hiding this comment.
Re-verified at f3d7547 (rebased onto main@e4c4f73 today). My 09-16 check was at efc08de, so these numbers are fresh.
The regression test does its job. Reverting the one-line change, test_brotli_export_starts_before_the_whole_ring_is_read fails with assert 4 == 1 — "Brotli must emit after the first source chunk". Restored, the file is 6 passed. uv run --group dev just check exits 0 on this head: 783 passed, 1 skipped.
Measured on a real export — 6,000,000 B of /r/lobby/export (18,498 records) fed through the actual ASGI app in 65,536-byte EXPORT_CHUNK blocks:
Accept-Encoding: br |
first body message | source consumed first | body msgs | total |
|---|---|---|---|---|
without streaming=True |
1,002,606 B | 38 of 92 chunks (2,490,368 B) | 3 | 2,401,671 B |
with streaming=True |
29,414 B | 1 of 92 chunks (65,536 B) | 93 | 2,410,729 B |
Cost: +9,058 B, +0.377%. That is middleware against middleware. A comparison against one-shot brotli.compress() suggests ~9%, but the middleware never does that, so that figure does not apply here.
The buffering threshold is output-side. Without the fix the encoder emits in ~1 MB compressed blocks, so time-to-first-byte is however much input it takes to fill one: 2,490,368 B on this sample, 3,735,552 B on the production export in #861. Same behavior, not two different numbers.
gzip was never affected, so the brotli-only test is correctly scoped. Same harness with Accept-Encoding: gzip: 1 of 92 chunks to first byte both with and without the change, 93 messages either way. streaming=True costs gzip +1,414 B (+0.054%) and changes nothing else.
No blockers.
## What Cuts **0.14.2**. Like #893, it dates the CHANGELOG section, bumps the version in the files the service, the MCP wrapper and the worker share, and re-ratchets the measured block of `sz-baseline.json`. No cap moves. ## Why **Main CI is red.** Since #163, every push to main fails `sz.py --check` (`core/limit.py 183 -> 186`, under its cap of 193). Because that step fails, the steps after it are skipped on main: the example e2e, the MCP wrapper build, and the image build and smoke. This PR's CI runs those steps against #163 and #126 for the first time. There are three fixes since v0.14.1, all PATCH, and nothing on the HTTP surface moves: - #865: a brotli `/r/<room>/export` streams again instead of buffering to near-EOF. - #163: the per-IP token bucket's read-modify-write runs under a lock, so concurrent threadpool requests can't overspend it. - #126: room and note readers treat a file reaped between the check and `open()` as absent, where they used to return a 500. After merge: tag `v0.14.2` on the squash commit, which publishes `ghcr.io/flop-labs/technocore-chat:0.14.2`. ## Checks - [x] `uv run coverage run -m pytest tests -q && uv run coverage report`: 812 passed, 98.08% - [x] `uv run ruff check . && uv run ruff format --check .` and `uv run ty check`: clean - [x] `uv lock --check` (root, and `mcp/worker` after `uv build --project mcp` built the 0.14.2 wheel), `sz.py --caps`, `sz.py --check`, and both `release.yml` gates - [x] Docs that would now be wrong are updated: `CHANGELOG.md` is the change - [x] New surface on a world-writable service: nothing new
Summary
application/x-ndjsonwithstreaming=Trueinstarlette-compress.Fixes #861.
Why this matters
/r/<room>/exportis aStreamingResponseemitted in 64 KiB chunks. With the existing registration, Brotli's non-streaming path buffered the export until 3,735,552 of 4,180,345 bytes had been consumed in the measured corpus. That delays the first byte, increases time-to-first-byte, and defeats back-pressure for the largest response lane.streaming=Truepreserves incremental delivery while keeping the byte-exact export payload unchanged.Evidence
6 passed in 3.76s776 passed, 1 skipped in 208.64s97.97%uv run python bench/compression.py): streamed br-41,189,695bytes at66.5 MB/s, versus gzip-41,463,246bytes at53.8 MB/s.ruff check,ruff format --check, andty checkpass;git diff --checkpasses.Known repository check
uv run sz.py --checkcurrently reports pre-existingcore/app.py 1020 -> 1023growth on upstreammain@542790b(the merged compression PR) againstsz-baseline.json; this change does not add further measured code lines beyond that upstream state. I did not modify the maintainer-regenerated baseline.