[tinker] Keep an undelivered sample result alive for the SDK's retry - #13
Closed
avigyabb wants to merge 1 commit into
Closed
[tinker] Keep an undelivered sample result alive for the SDK's retry#13avigyabb wants to merge 1 commit into
avigyabb wants to merge 1 commit into
Conversation
retrieve_future started the short retrieved-TTL clock as soon as the response object was built, even for a poll the SDK had already abandoned after its 45s client timeout. uvicorn drops the send to a dead client silently, the sweeper evicted the result 120s later, and the SDK's retry of the same request_id got 404 "Future not found", which the SDK treats as fatal. This is the 128x128 failure seen against j316chuck#18. Start the clock only if request.is_disconnected() is false, and raise the retrieved TTL to 300s so it outlasts the SDK's worst-case re-poll gap (45s timeout plus up to 30s backoff, twice). tests/tinker/ test_retrieve_future_lost_response.py reproduces the chain under a real uvicorn socket; it fails without this change and passes with it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
This was referenced Sep 4, 2026
avigyabb
marked this pull request as ready for review
September 4, 2026 01:15
Owner
Author
|
Moved upstream: NovaSky-AI#2162 |
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.
Stack 3/7. Fixes the 128x128
404 Future not foundseen against j316chuck#18.Chain. The SDK polls
retrieve_futurewith a 45 s client timeout and gives up; the result lands afterwards; the abandoned handler wakes, builds a response nobody receives (uvicorn drops the send to a dead client silently) and starts the short retrieved-TTL clock; the sweeper evicts the result 120 s later; the SDK's retry of the same request_id gets 404, which the SDK treats as fatal.Fix. Start the retrieved clock only if
request.is_disconnected()is false, and raise the retrieved TTL to 300 s so it outlasts the SDK's worst-case re-poll gap (45 s timeout + up to 30 s backoff, twice).tests/tinker/test_retrieve_future_lost_response.pyreproduces the chain under a real uvicorn socket with shortened TTLs; it fails onmainand passes here. A second test checks a delivered result still expires on the short clock, so memory stays bounded.Alternative considered: j316chuck#19 drops the retrieved clock and keeps every result for 2048 s. That also fixes the 404 but retains ~35 minutes of results regardless of delivery; with long-output rollouts (hundreds of KB per result) that is tens of GB. Verified at scale: 131072 requests with 5 s engine queueing and 224k SDK-style abandoned polls completed with zero 404s.
Stack
🤖 Generated with Claude Code