Repository navigation
[API server][Bug fix] Release consumed streaming output chunks - #844
Conversation
Drain pending output lists under the request lock so consumed chunks can be released before the stream finishes. Preserve profiling totals with per-modality counters and add regression coverage for reclamation, final flushing, collection, and errors.
arun-elr
left a comment
There was a problem hiding this comment.
Thanks @Achuthantm for picking up #678! Nice to see the LingBot RSS growth between chunks 4 and 63 go from about 222 MiB to 0.
I ran it on the PR merged into current main and found nothing blocking:
- Drain vs the producer, plus abort, timeout, an error after some chunks, an engine failure and conductor death mid-stream, all behave as on main (CPU probes driving the real message loop).
- Non-streaming collection still returns every chunk, and the
--log-statsoutput totals match main, including chunks that land after an error. - The Rust frontend's bridge also reads through
iter_result_chunks, so it gets the fix too. With a realAPIServerbehind both the Python app andmstar-server, a 120-chunk stream held at most 2 unsent chunks (main held all 120) for text, audio, image and video_frame. - CPU Core: 5,995 passed. 3 signal-timing tests flaked under xdist and pass on their own on both main and this branch.
One thing to line up:
- #593 (full duplex 3/6) rewrites the same drain lines and counts profile outputs at delivery instead, so whichever lands second will conflict. Counting on arrival as done here keeps main's numbers, so this version is probably the one to keep in the rebase.
Since this changes how every streamed request hands chunks to the frontend, could you also run the e2e regression matrix base vs head with alternating fresh servers (ABBA): Orpheus streaming TTS, Pi0.5 (multi-stage graph) and LingBot-World video streaming, and paste the summary into the PR? I swapped Z-Image for LingBot because /v1/images/generations submits with streaming=False and never reaches the new drain, and LingBot is the #678 case anyway.
|
@arun-elr Thanks for the thorough review! I ran the requested fresh-server ABBA (base-head-head-base) e2e matrix on one A40 with torch All four fresh-server runs completed successfully for each model: Orpheus, Pi0.5, and LingBot-World. All 60 requests completed (48 measured + 12 warmups). Profiling output counts and byte totals matched across base/head. LingBot outputs were byte-identical. Orpheus and Pi0.5 showed output variation comparable to repeated-base runs.
Noted the #593 overlap: I’ll preserve arrival-side accounting when resolving the conflict. |
|
Added it to the PR description, thanks |
|
@dghosef Thanks for the review! The CI workflow is still awaiting maintainer approval. Could you approve it when you get a chance? |
4a82fb5 to
f7026a5
Compare
|
Thanks @Achuthantm |
Conflicts: - mstar/model/base.py: kept this branch's ClientChunk; dropped PrefixStream, which main replaced with KVLabel/PrefixKeys (#832/#834). - mstar/api_server/data_worker.py: imports take ClientChunk plus main's PrefixKeys/keyed_labels; this branch's _load_media refactor kept with main's _prefix_streams -> _keyed_labels rename; _finish_reading drains when the rid is already draining (this branch) or still has a failed read's transfers in flight (main #419), docstring merged. - mstar/api_server/entrypoint.py: PendingRequest takes main's output_totals (#844), which counts outputs as they are accepted; this branch's `delivered` counter and _count_outputs, the same fix for the same leak, are deleted. Stream drain keeps this branch's last_activity read with the swap-out; the done path clears req.chunks as main does. /generate/ws: main's empty-id-as-None and check_request_id (#618) move into the receive loop beside this branch's isinstance check, since live-call routing reads the id there; the error reply keeps this branch's call/turn release with main's client_error_text scrubbing (#646), under this branch's `payload` name. - mstar/conductor/conductor.py: imports from both; worker graphs from main's validate_deployment (#640), then this branch's _find_client_streams. - test/modular/test_kv_prefix_request_keys.py: kept this branch's get_partition_topology stub and main's kv_labels. Adapted to main (no conflict markers): - test/modular/test_client_stream_api.py: stub renamed prefix_key_streams -> kv_labels; the tensor-manager fake gains take_failed_reads; the bad-append test expects #646's "internal error" text in place of the model's exception; test_a_streaming_request_lets_go_of_the_chunks_it_has_delivered dropped, as main's test_api_result_delivery.py covers the release and the totals. - test/modular/test_request_id_limit.py: the fake server gains takes_stream_input, submission_key and abort_submission, which this branch's /generate/ws reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's #844 replaced the consumed_chunks cursor with draining req.chunks; the sink relay takes its requests' chunks the same way, up to an error chunk, which stays for the handler. Main's inline-token test uses the per-frame InlineTokens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rame, raw token bytes, batched Rust-frontend relay (#849) * api_server: one data-worker item and one model call per RESULT_TOKENS frame Inline decode tokens cost the API server's Python threads about 29 us per token, most of it plumbing around a 2 us detokenize: one queue item, one tensor, one drop_stop_tokens and one decode per token. * The message thread hands a frame's values for the requests still pending to the data worker as one InlineTokens item (PreprocessWorker .new_result_tokens), with the same per-request accounting as before. * The data worker postprocesses the frame with one Model.postprocess_inline(values, modality, request_kwargs) call, then queues each request's chunk through the reorder buffer as before. If the call raises, it is retried row by row so only the failing request fails. * Model.postprocess_inline defaults to a one-element tensor plus postprocess per value, which is what every model got before. * Qwen3.5 overrides it with one raw-bytes lookup per value through its TextChunkDetokenizer, without a tensor per value. Stop tokens are dropped unless ignore_eos, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit fee943ac3289da748f7f94ac59c4955fc0585e43) * benchmark: API server output-path harness (stages, threaded, HTTP e2e with a fake engine) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 4cd92c4aa919f2fcc4d4f2776e34b2cbe316bf9e) * Rust frontend: relay a message pass's chunks to mstar-server as one bridge message With --rust-frontend, every chunk cost the API server's event loop a handler wake-up, a msgpack dict and a ZMQ send: 13 us per token, the largest Python cost left on the output path. * APIServer.relay_chunks_to(key, sink, tag) attaches a ChunkSink to a submission. Once per pass, the message thread hands each sink the chunks it drained for the sink's requests with one call_soon_threadsafe, instead of waking each request's handler per chunk. * The handler (iter_result_chunks) still owns completion, errors and the timeout. The sink takes chunks up to an error chunk; the handler yields what is left, through the same consumed_chunks index under request_lock, so the two never duplicate or reorder a request's chunks. * RustFrontendBridge is a sink: one {t:"chunks", subs, modalities, payloads, metadatas} message per pass. mstar-server delivers each entry to its submission as a chunk. A submit carries accepts_chunks, so an older mstar-server binary keeps getting one chunk message per chunk. The Python frontend is unchanged: nothing attaches a sink there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d098ff38d478aa94ab29169fd52794b5cea426e7) * API server: name the per-sink relay batch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * API server: note why inline tokens carry no loop indices; docs: when the Rust frontend matters Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: drop the tests that restate the code or a copy of main's Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * worker, Python runtime: no debug-log formatting while debug is off The INPUT_SIGNALS handler formatted every message's edge list, the Python runtime three edge lists per routed rid, and two per-step f-strings their request-id lists, all for logger.debug calls that print nothing at INFO. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * transports: a read of signal-only edges returns before the stream context A streamed token's INPUT_SIGNALS also calls start_read_tensors for its (empty) non-streamed edges; the SHM and arena transports entered the H2D stream context (and the arena swept for dead peers) before finding nothing to read: about 15 us per message, ~8% of the SNAC worker's wall time with Orpheus on 2 GPUs at c=64. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * api output on main: the sink relay drains req.chunks (#844) Main's #844 replaced the consumed_chunks cursor with draining req.chunks; the sink relay takes its requests' chunks the same way, up to an error chunk, which stays for the handler. Main's inline-token test uses the per-frame InlineTokens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * API server: a model declares its text output; a failed relay fails its requests - Model.text_output() returns a TextOutput (detokenizer, stop ids), and the base derives postprocess's text path and postprocess_inline from it, so every text model gets the per-frame path without a tensor per value. Qwen3.5, Whisper, Qwen3-ASR, BAGEL and Qwen3-Omni declare it; Qwen3.5's postprocess_inline override and the per-model detokenizer caching go. - The detokenizers memoize a token's bytes (token_bytes), which the inline path reads. - A chunk sink's relay that raises fails the requests whose chunks it held, instead of the loop logging it and the requests finishing without them. - docs: the postprocess hooks are optional, and TokenBytes is TextChunkDetokenizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * API server: an abort wakes the request's stream handler abort_submission dropped the request without waking its handler, which noticed only at its next 50 ms backstop wake. The Rust bridge aborts a submission its frontend dropped this way. The backstop stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * model utils: import safetensors where it is used mstar.model.base now imports mstar.model.utils, and safetensors is not a core dependency, so a core install (the wheel tests, rust-transport) failed to import mstar.model.base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: the inline chunk-count stub takes a frame's InlineTokens Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #678.
Streaming requests kept already-delivered chunks until completion, so API-server
memory grew with the total output. Drain the pending list under
request_lockby transferring ownership to the consumer, allowing consumed payloads to be
released before the request ends.
Track per-modality counts and byte totals separately to preserve profiling.
Add regression coverage for reclamation, final flushing, non-streaming
collection, profiling, and errors.
How was it tested?
ruff check .,git diff --check, and local Maclintandcpu-smoke:passed.
and torch 2.9.1/CUDA 13.0. The new tests confirm consumed chunks are released
before request completion and catch the original bug.
collection-order issue required running the affected module serially;
it passed. The TP simulation also passed separately.
1,021 frames / 64 chunks per request, two fresh-process runs per arm.
Median sampled peak API RSS fell from 1,358.6 to 1,139.8 MiB.
Between chunks 4 and 63, baseline RSS grew 221.8–224.7 MiB; the fix
showed 0.0 MiB growth. Queue samples confirmed consumed history was
released, and full-stream hashes and profiling totals matched.
The API, Linux CPU Core, and A40 results were recorded on 2026-10-22 for the
changes in this PR. Slow consumers can still accumulate unread output;
these measurements do not establish a hard memory cap or a speedup.
Checklist
ruff check .passesAI assistance was used in developing this change.