fix(orchestrator): mint distinct message IDs for cancel re-publishes - #505
Open
behinddwalls wants to merge 1 commit into
Open
fix(orchestrator): mint distinct message IDs for cancel re-publishes#505behinddwalls wants to merge 1 commit into
behinddwalls wants to merge 1 commit into
Conversation
behinddwalls
marked this pull request as ready for review
August 4, 2026 01:53
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 4, 2026 01:58
733a5fd to
f2e6633
Compare
This was referenced Aug 4, 2026
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 4, 2026 05:11
f2e6633 to
dda3dd3
Compare
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 5, 2026 03:15
dda3dd3 to
96feaf5
Compare
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 5, 2026 03:26
96feaf5 to
d450509
Compare
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 5, 2026 03:38
d450509 to
9227c40
Compare
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 5, 2026 03:44
9227c40 to
dbd61ed
Compare
## Summary ### Why? The cancel controller published its speculate hand-off with the bare batch ID as the message ID. The queue deduplicates on (topic, partition key, message ID) against every row it has not garbage-collected yet, consumed ones included, so a redelivery's re-publish for the same batch was a silent no-op — leaving a batch stuck Cancelling with nothing driving it to terminal. Same class of bug as b1f5795 in stovepipe. ### What? `publishBatchID` now goes through `submitqueue/core/publish`: `publish.UniqueID` mints a distinct message ID per publish (and documents the dedup rule once), and `publish.Message` owns the registry-lookup plumbing the controller previously hand-rolled. The batch-path test now asserts the payload still carries the batch ID while the message ID is distinct per publish, and the multi-batch tests assert on the payload's batch ID for the same reason. ## Test Plan ✅ `bazel test //submitqueue/orchestrator/controller/cancel/...`
behinddwalls
force-pushed
the
preetam/speculation-cancel-msgid
branch
from
August 5, 2026 04:11
dbd61ed to
baf15bf
Compare
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
1 participant
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 cancel controller published its speculate hand-off with the bare batch ID as the message ID. The queue deduplicates on (topic, partition key, message ID) against every row it has not garbage-collected yet, consumed ones included, so a redelivery's re-publish for the same batch was a silent no-op — leaving a batch stuck Cancelling with nothing driving it to terminal. Same class of bug as b1f5795 in stovepipe.
What?
publishBatchIDnow goes throughsubmitqueue/core/publish:publish.UniqueIDmints a distinct message ID per publish (and documents the dedup rule once), andpublish.Messageowns the registry-lookup plumbing the controller previously hand-rolled. The batch-path test now asserts the payload still carries the batch ID while the message ID is distinct per publish, and the multi-batch tests assert on the payload's batch ID for the same reason.Test Plan
✅
bazel test //submitqueue/orchestrator/controller/cancel/...Issues