fix(dflash): abandoned stream drain - #231
Merged
Merged
Conversation
jkyamog
force-pushed
the
fix-abandoned-stream-drain
branch
from
May 20, 2026 08:32
9e12596 to
d11b0aa
Compare
Contributor
Author
|
CAVEAT: Also I only tested this behavior in deployment together with PR #230. |
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.
Fixes a Python
server.pystream-synchronization bug where an SSE client can disconnect before the dflash daemon has emitted its-1sentinel. In that case the daemon continues writing tokens to the sharedr_pipe, but the Python streaming generator exits and releasesdaemon_lock. The next request can then consume stale tokens from the abandoned request, producing impossible timings and wrong responses.This PR keeps the daemon protocol synchronized by draining abandoned streams to the sentinel while still holding
daemon_lock.Observed Failure
From a real Hermes run:
After that, later requests showed impossible near-zero wall time and consumed output that belonged to earlier daemon work:
That is not realistic model performance. It is stale token-pipe data from a previously abandoned stream.
Root Cause
Streaming requests share one daemon token pipe (
r_pipe) protected bydaemon_lock.Normal path:
r_pipe.-1.timing["daemon_done"] = True.daemon_lock.Failure path:
-1.r_pipe.daemon_lock.Fix
When a streaming response exits before
timing["daemon_done"]:r_pipeto the daemon sentinel using_drain_until_sentineldaemon_lockThis does not cancel in-flight GPU work. It is a synchronization fix: the next request starts from a clean daemon pipe instead of inheriting stale output.
Logging And Timing
The abandoned-stream log should stay at warning level, not debug. A client disconnect is normal, but the server reaching the end of an SSE generator before the daemon sentinel is a protocol synchronization hazard: if it is not handled, the next request can read stale token IDs from the previous request.
This PR does not change timing math and does not count prefix-cache time differently. The low/impossible timings disappear because the next request no longer consumes already-buffered tokens from an abandoned prior stream. Normal cached requests should still report fast prefill when a prefix snapshot is restored, but they should not show impossible decode rates caused by stale pipe data.
The drain log includes how many stale daemon tokens were consumed during cleanup, which is enough evidence for this specific synchronization fix without adding broader empty-response diagnostics to this PR.
Relationship To Existing Issues
Related issue:
Related but distinct issue:
#216 fixed RESTORE prompt-delta correctness. This PR addresses a different stale-output path: abandoned streaming responses leaving unread daemon tokens in
r_pipe.