test(e2e): prove a dependent batch is woken by the merge ahead of it - #576
Merged
Merged
Conversation
mnoah1
approved these changes
Aug 12, 2026
behinddwalls
force-pushed
the
preetam/messagequeue-e2e-dependent-wake
branch
from
August 12, 2026 18:44
dc085b9 to
bcb260d
Compare
## Summary ### Why? The parent change fixes a dropped wake-up: a merged batch fans out to speculate so its dependents can re-plan, and that message used to reuse the bare batch ID the batch controller had already published to the same topic and partition at creation. The queue deduplicates against rows it has not collected yet, consumed ones included, so the fan-out was reported as a success, stored nothing, and never arrived. That fix shipped with unit coverage on the message ID and integration coverage on the queue semantics, but nothing exercised the path the bug actually broke. It is also a path that hides easily: any other event re-plans the queue and moves the dependent along, so a naive two-request test passes with or without the fix. ### What? A new e2e case isolates the fan-out as the only possible wake-up, following the stop → observe → start shape `TestCancel_CaughtPreBatch_NeverLands` already uses: 1. Close the `runway-merge` gate for the queue before landing, so the lead batch cannot complete its merge. 2. Land the lead; wait for its merge to park, keyed by the lead's batch ID. 3. Land the dependent. Its batch serializes behind the lead's, which is in-flight (`Merging` is a dependency state). 4. Wait for the dependent to reach `speculated` — its speculative build has already passed, so its own build signals are finished and nothing else will wake it. 5. Open the gate. The lead merges and fans out. The dependent reaching `landed` is then attributable to the fan-out alone. Supporting changes: `e2e-chain-queue` is registered in `queues.yaml`. It is deliberately absent from the orchestrator's per-queue profiles so it falls through to the baseline profile and its `all` conflict analyzer, which serializes every new batch behind every in-flight one — that is what builds the chain. A new `awaitBatchID` harness helper resolves a request's batch ID from the operating store, since merge messages are keyed by batch rather than by the sqid a test holds. ## Test Plan - ✅ `bazel test //test/e2e/...` — 3 suites, including the new case (~33s) - ✅ `make lint-license`, `make lint-message-id`, `make lint-queue-shard` - **Confirmed the test fails against the unfixed code.** Reverting the parent's `mergesignal` message ID to the bare batch ID leaves the dependent stuck at `speculated` and the suite runs to Bazel's timeout (`TIMEOUT in 240.3s` with `--test_timeout=240`); with the fix it passes in 33s. A stalled pipeline surfaces as a test timeout rather than an assertion failure, which is how this harness reports non-convergence — `pollUntil` has no deadline of its own by design, so Bazel's timeout is the only one.
behinddwalls
force-pushed
the
preetam/messagequeue-e2e-dependent-wake
branch
from
August 12, 2026 19:00
bcb260d to
20e0c3d
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 12, 2026
behinddwalls
added a commit
that referenced
this pull request
Aug 13, 2026
## Summary ### Why? `TestDependentBatch_IsWokenByTheMergeAhead` parks the lead batch's merge behind a closed gate, waits for the dependent to reach `speculated`, and only then opens the gate. That ordering encodes the old meaning of `speculated` — a build passed on a path still consistent with how its dependencies are resolving — which a batch could reach while its dependency was still outstanding. Holding `speculating` until the batch can be sent to merge removes that resting point. A dependent blocked on the parked lead now stays `speculating`, and `speculated` arrives only once the lead has merged and the dependent is itself cleared to merge. So the test waits for a status that cannot arrive until it opens the gate, and it does not open the gate until that status arrives. The suite runs to Bazel's timeout. The two changes had not met before: #576 landed on main after this branch was cut, so CI had never run them together. ### What? The observation step waits for the `waiting` event instead of the `speculated` status. It is the same fact the test was reaching for — the dependent has passed its own build and only the lead is outstanding — expressed as the signal that now carries it, and reachable while the lead is still parked. Nothing else moves. The gate still opens next, and the lead and the dependent are still asserted to land, so the dependent's wake-up remains attributable to the fan-out alone. ## Test Plan ✅ `bazel test //test/e2e/submitqueue:go_default_test` — passes in 120s, against a 300s timeout before ✅ `bazel test //submitqueue/... //platform/...` — 69 tests pass
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
2 participants
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
Why?
The parent change fixes a dropped wake-up: a merged batch fans out to speculate so its dependents can re-plan, and that message used to reuse the bare batch ID the batch controller had already published to the same topic and partition at creation. The queue deduplicates against rows it has not collected yet, consumed ones included, so the fan-out was reported as a success, stored nothing, and never arrived.
That fix shipped with unit coverage on the message ID and integration coverage on the queue semantics, but nothing exercised the path the bug actually broke. It is also a path that hides easily: any other event re-plans the queue and moves the dependent along, so a naive two-request test passes with or without the fix.
What?
A new e2e case isolates the fan-out as the only possible wake-up, following the stop → observe → start shape
TestCancel_CaughtPreBatch_NeverLandsalready uses:runway-mergegate for the queue before landing, so the lead batch cannot complete its merge.Mergingis a dependency state).speculated— its speculative build has already passed, so its own build signals are finished and nothing else will wake it.The dependent reaching
landedis then attributable to the fan-out alone.Supporting changes:
e2e-chain-queueis registered inqueues.yaml. It is deliberately absent from the orchestrator's per-queue profiles so it falls through to the baseline profile and itsallconflict analyzer, which serializes every new batch behind every in-flight one — that is what builds the chain. A newawaitBatchIDharness helper resolves a request's batch ID from the operating store, since merge messages are keyed by batch rather than by the sqid a test holds.Test Plan
bazel test //test/e2e/...— 3 suites, including the new case (~33s)make lint-license,make lint-message-id,make lint-queue-shardmergesignalmessage ID to the bare batch ID leaves the dependent stuck atspeculatedand the suite runs to Bazel's timeout (TIMEOUT in 240.3swith--test_timeout=240); with the fix it passes in 33s.A stalled pipeline surfaces as a test timeout rather than an assertion failure, which is how this harness reports non-convergence —
pollUntilhas no deadline of its own by design, so Bazel's timeout is the only one.Stack