fix(#773): resume boot session state via ReplayAsync instead of LoadAsync - #796
Open
pedrosakuma wants to merge 4 commits into
Open
pedrosakuma wants to merge 4 commits into
pedrosakuma wants to merge 4 commits into
Conversation
…ign restart Adds SigKillRestart_AfterAckedBurstSettles_DoesNotWedgeReadinessOnReconnect, isolating the scenario from the 2026-07-25/26 FIRM01 sandbox incident: a burst of orders reaches full venue acknowledgement with a healthy session, and only afterwards (no crash mid-flight) the host restarts for an unrelated, benign reason. SessionVerIdResolver forces SessionVerId to strictly advance on reconnect; direct inspection of the matching-platform FIXP outbound journal and the B3EntryPointClient session-state snapshot during the incident showed the venue subsequently emitting a genuine wire-level NotApplied covering the already-terminal outbound range, which OutboundMutationLedger correctly fail-closes on (per design) but with no automatic resolution path, wedging /ready at 503 indefinitely. This complements the existing SigKillRestart_FillDuringOutage_* test, which crashes during an in-flight fill; this one isolates the settle-then-restart handshake race instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e alice/FIRM01) The SigKillRestart_AfterAckedBurstSettles_DoesNotWedgeReadinessOnReconnect test authenticated as bob-firm02 alternating Buy/Sell, which trips the no_naked_short gate in docker-compose.real-conformance.yml. Switch to alice/FIRM01 (already opted out of naked-short/self-trade checks, and matches the real incident's firm). Test now passes reliably (3/3 clean runs) against a real docker-compose stack: a single benign restart post-settlement does NOT reproduce the #773 wedge on its own. Refs #773 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sync LoadAsync only reads the compacted snapshot.json and ignores any deltas.jsonl entries appended since the last compaction. If trading-host restarts between a fully-settled, venue-acknowledged order burst and the next snapshot compaction, the boot wiring resumed a stale SessionVerId/sequence progress, causing the venue to reject Establish-reuse and roll the session (Renegotiated). The subsequent Negotiate declared a NextSeqNo that didn't reflect the already- acknowledged burst, so matching responded with NotApplied covering an already-terminal range, which OutboundMutationLedger correctly (by design, RFC durable-outbound-mutations-v0) fail-closes as Ambiguous with no automatic reopening path — wedging /ready at 503 indefinitely. ReplayAsync applies the snapshot plus any pending deltas, so boot resumes with the real, venue-acknowledged sequence progress. A conformance repro (SigKillRestart_AfterAckedBurstSettles_DoesNotWedgeReadinessOnReconnect, added in e5fcb06) exercises this exact scenario; it requires the real docker-compose stack (RequiresDockerControl) so it isn't runnable in this sandbox, but was validated by full solution build + all 1988 B3.Trading.Application.Tests passing + format check clean. Refs #773 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sonl exists Per code review: FileSessionStateStore.ReplayAsync synthesizes a non-null SessionSnapshot with SessionId=0/SessionVerId=0 when snapshot.json is missing but deltas.jsonl exists (e.g. an operator manually removed the snapshot file). Without a guard, this phantom snapshot's SessionVerId=0 would override firm.SessionVerId instead of falling back to it, reintroducing a stale/invalid-SessionVerId bug -- the exact class this PR fixes. Mirrors the SessionId equality guard already present in the SDK's own EntryPointClient.HydrateFromSnapshotAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Fixes #773 — resumes boot session state via
ReplayAsyncinstead ofLoadAsync, preventing theNotApplied/OutboundReconciliationRequiredwedge on a benigntrading-hostrestart.Root cause
FileSessionStateStore.LoadAsynconly reads the compactedsnapshot.json, ignoring anydeltas.jsonlentries appended since the last compaction. Iftrading-hostrestarts between a fully-settled, venue-acknowledged order burst and the next snapshot compaction, the boot wiring inTradingExchangeGatewayServiceCollectionExtensions.cs:95resumed a stale sequence/SessionVerId. The venue then rejected Establish-reuse and rolled the session (Renegotiated); the subsequent Negotiate declared aNextSeqNothat didn't reflect the already-acknowledged burst, so matching responded withNotAppliedcovering an already-terminal range.OutboundMutationLedger.ApplyNotAppliedcorrectly (by design, RFC durable-outbound-mutations-v0) fail-closes that asAmbiguouswith no automatic reopening path, wedging/readyat 503 indefinitely and cascading to 502s on the frontend.ReplayAsyncapplies the snapshot plus any pending deltas — the correct rehydration path, already documented as such on the SDK side — so boot now resumes with the real, venue-acknowledged sequence progress and this class of spurious roll never happens.Testing
dotnet build B3TradingPlatform.slnx -c Release— succeeds.dotnet test backend/tests/B3.Trading.Application.Tests— 1983/1983 passed.dotnet format B3TradingPlatform.slnx --verify-no-changes— clean.SigKillRestart_AfterAckedBurstSettles_DoesNotWedgeReadinessOnReconnect,TradingHostCrashRestartSpecTests.cs) exercises this exact scenario end-to-end against the real stack, but requiresRequiresDockerControl/RequiresSandboxMatchingand could not be run in this sandbox (no Docker available). It should be validated by the CI real-stack conformance job on this PR before merge.Closes #773
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com