Repository navigation
server2: count response-first waiters before a batch is ready - #2903
Merged
Merged
Conversation
CI run 42379684133 (PR #2872, 2026-11-07) failed full_response::empty_weather_store_serves_seven_days_before_raw_publication_and_replays_identically: one raw publication before the body terminal. Its trace shows a group published 2 ms into the owned flight that acquired it, while the request was still running. Writer::acquire counted a waiter only by its watch receiver, and run() asked receiver_count() after sending Ready: a waiter that took the batch and returned before that check, or a creating caller not yet polled again when an instant provider was done (it subscribed after spawning the job), left the batch looking unwaited, and NoWaiters released it, so the response's own raw was stored before the response finished (against provider -> assemble -> respond -> store). The caller now subscribes before the job is spawned (or under the table lock when it joins), and the waiters are counted before Ready is sent. tests/response_first.rs a_creating_waiter_holds_its_instant_provider_until_the_body_ends (2,000 scoped instant acquisitions) failed 17/20 runs with 30 busy loops on 10 cores (NoWaiters releases before the body ended) and passes 20/20 under the same load. Same change as e708d12 on #2872. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Cross-provider review (Codex gpt-6.1-sol, high), round 1: CHANGES_REQUESTED at 0fc562f. Required
Recommended
The same change goes into #2872. |
… (RFW-zero-count-join)
run() counted the receivers, then announced Ready and decided NoWaiters,
without the table lock under which callers join entries. A response that
joined after its creator was cancelled (count 0) but before Ready got the
batch while the stored 0 released it as unwaited, so its raw was
published while its body was still being sent.
Who waits is now settled under the table lock together with the Ready
announcement and the NoWaiters decision. Publisher::checkpoint is a test
seam (a default that is ready at once; no setting): the writer awaits it
when a batch is valid and not yet Ready. The new
a_response_joining_before_ready_holds_a_cancelled_creators_batch forces
that order (creator cancelled, batch valid, a new response joins, then
Ready): with the count taken before the checkpoint it failed 3/3
("released before the body ended", [NoWaiters]); it passes 5/5.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heduling) The regression test for the waiter count ran 2,000 instant acquisitions and relied on the scheduler to preempt at the right step, so the old code could pass it, and its "nothing released" check could trip on the 10 s dormancy cap of a slow runner. Two more Publisher::checkpoint points (Created: the creating caller has started its job; Announced: the batch is Ready and its waiters settled) let tests force the orders: a_creator_not_yet_waiting_when_its_batch_is_ready_holds_it and a_waiter_that_takes_the_batch_at_once_holds_it. With the creator subscribing after Created, or the waiters counted after Announced (the orders before 0fc562f), each failed 3/3 ("released before the body ended", [NoWaiters]); they pass now. Their checks run right after the waiter returns, under the 10 s cap. The mass run is an ignored stress check that fails only on NoWaiters (passed 5/5 with 30 busy loops on 10 cores). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hwanjjang
added a commit
that referenced
this pull request
Nov 7, 2026
… (RFW-zero-count-join) run() counted the receivers, then announced Ready and decided NoWaiters, without the table lock under which callers join entries. A response that joined after its creator was cancelled (count 0) but before Ready got the batch while the stored 0 released it as unwaited, so its raw was published while its body was still being sent. Who waits is now settled under the table lock together with the Ready announcement and the NoWaiters decision. Publisher::checkpoint is a test seam (a default that is ready at once; no setting). The new a_response_joining_before_ready_holds_a_cancelled_creators_batch forces that order; with the count taken before the checkpoint it failed 3/3 ([NoWaiters]). Same writer and test code as 7d27588 on fix/2614-response-first-waiters (#2903). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hwanjjang
added a commit
that referenced
this pull request
Nov 7, 2026
…heduling) The regression test for the waiter count relied on the scheduler to preempt at the right step over 2,000 instant acquisitions, and its "nothing released" check could trip on a slow runner's 10 s dormancy cap. Two more Publisher::checkpoint points (Created, Announced) let a_creator_not_yet_waiting_when_its_batch_is_ready_holds_it and a_waiter_that_takes_the_batch_at_once_holds_it force the orders; each failed 3/3 with the old order ([NoWaiters]). The mass run is an ignored stress check that fails only on NoWaiters. The spec's RESPONSE-FIRST criterion names the forced-order tests. Same writer and test code as 03f7b57 on fix/2614-response-first-waiters (#2903). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Cross-provider review (Codex gpt-6.1-sol, high), round 2: PASS at 03f7b57. RFW-zero-count-join is resolved. Recommended
|
Collaborator
Author
|
Final readiness (pre-merge): merge-ready at ffe9faf, which includes master c393ce2. CI passes, and the cross-provider review passed (Codex, two rounds; one remaining recommendation not selected, with the reason in the review record). The PR has no auto-merge and no merge queue. Merging is AK's decision. #2872 carries the identical writer change. |
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.
Summary
The response-first writer could publish a response's own raw group before the response body ended. That breaks the agreed order: provider, assemble, respond, then store.
CI run 42379684133 (PR #2872, 2026-11-07) failed
full_response::empty_weather_store_serves_seven_days_before_raw_publication_and_replays_identicallywith "no raw publication before the body terminal". Its trace shows a group published 2 ms into the owned flight that acquired it, while the request was still running.Cause (
server2/src/storage/response_first.rs)Writer::runsentReadyand only then askedreceiver_count(). A waiter that took the batch and returned in between made the batch look unwaited, and it was released asNoWaiters.Writer::acquirespawned the acquisition job before the creating caller subscribed. With a provider that answered at once, the batch could be ready before the caller counted.Fix
Readyis sent.Readystill releases the batch asNoWaiters, as before.Checks
tests/response_first.rs::a_creating_waiter_holds_its_instant_provider_until_the_body_endsruns 2,000 scoped acquisitions with instant providers. With 30 busy loops on 10 cores it failed 17 of 20 runs before the fix and passed 20 of 20 after.clippy -D warningsare clean,response_firsthas 18 passed, and the domestic full-cycle test passes.KMA-RESPONSE-FIRST-WAITERS.json) and the artifact policy pass.#2872 carries the same change, declared under its own task, so its CI stops flaking on this test. This PR brings the fix to master on its own.
🤖 Generated with Claude Code