fix(gateway): recover shutdown-flushed messages - #105270
Closed
Gabrielnkl wants to merge 1 commit into
Closed
Gabrielnkl wants to merge 1 commit into
Gabrielnkl wants to merge 1 commit into
Conversation
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
Follow-up to #75536: make shutdown-flush recovery reliable on the current gateway implementation.
The original session-resolution issue identified in #75536 is still present on current
main, but the gateway has since changed and FIFO overflow events are now also persisted through the shutdown-flush path.This PR rebases the fix onto the current recovery path and adds two correctness fixes discovered while testing the current implementation:
MessageEventpayloads throughsession_key → session_idRoot cause
A real gateway
MessageEventdoes not containsession_id.During shutdown serialization, the event therefore produces a payload shaped like:
{ "session_key": "...", "data": { "text": "..." } }The recovery path previously required
data["session_id"].As a result:
The existing persisted
session_keyis sufficient to recover the session throughSessionStore.peek_session_id().Current-main overflow interaction
Since #100972, FIFO overflow events are also written to the shutdown spool.
Those events use the same recovery format, so they are subject to the same session-resolution problem.
This PR adds regression coverage for both normal pending-message flushes and overflow flushes using the real
MessageEventshape.Ordered replay
Shutdown-flush payloads can contain a persisted
seqrepresenting arrival order.The recovery path previously iterated spool files by filename. UUID-based filenames do not guarantee arrival order.
For example:
Recovery must produce:
rather than filename order.
The recovery path now sorts by:
Legacy payloads without these fields retain deterministic fallback ordering.
Failure isolation
Recovery should not allow one bad spool entry to prevent unrelated valid messages from being recovered.
The new behavior is:
A failed recovery never deletes its spool file.
Tests
Added regression coverage for:
MessageEventpayloads withoutsession_idFocused validation:
Current branch:
The full gateway suite was also exercised; unrelated pre-existing failures remain outside the shutdown-flush changes.
Relationship to #75536
#75536 identified and implemented the original
session_key → session_idrecovery fix.This PR carries that fix forward onto the current gateway architecture and adds:
Credit to #75536 for the original diagnosis and core session-resolution approach.
Related: #72680, #99882, #100972.