fix: don't advance the book-version chain when no frame is emitted (#115)#116
Merged
Conversation
) Root cause of the steady-state chain flaps (~4/market/min, zero cluster-side shed): a book mutation below the visible top-20 depth bumps the side versions, so encodeBookSnapshot proceeds, but the visible diff is empty — encodeBookDelta returns 0 and nothing is broadcast. Yet lastPublishedBookVersion advanced anyway, so the next visible delta named a fromVersion no consumer ever received. The gateway (correctly, per match#96 drop-don't-drift) flagged a chain break and froze the book until the next match#112 periodic resnapshot. Books were silently drifting through these same skips before match#96 exposed them. Fix: advance lastPublishedBookVersion only when a frame was actually encoded. An unsent version gap is fine — the next delta spans it (visible state to visible state), so the emitted stream stays chain-continuous by construction. Also (audit finding, same family): drainQueue() gave up on a message for a session after 3 backpressured offer retries with NO accounting — a real egress loss invisible to every existing drop counter. Now counted (match_egress_offer_giveups_total, should stay 0) with a rate-limited CRITICAL log. Regression tests: MarketPublisherBookChainTest reproduces the exact live break shape (delta fromVersion=2000 while the consumer holds 1000) and asserts chain continuity across mixed deep/visible flush cycles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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
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.
Fixes #115.
Root cause
MarketPublisher.encodeBookSnapshotadvancedlastPublishedBookVersion = collectedBookVersionunconditionally — even whenencodeBookDeltareturned 0 because the mutation happened below the visible top-20 depth (side version bumped, visible diff empty, nothing broadcast). The next visible delta then carried afromVersionno consumer ever received, which the gateway correctly treats as a chain break (#96): book flagged stale, frozen until the next periodic resnapshot (#112).This explains every piece of the live evidence:
deltaBidV == haveBidV, ask versions skipping by 4) — deep ask-side churn from the sim;match_dropped_market_msgs_total = 0on all nodes — nothing was dropped, nothing was ever sent;No raw-frame capture needed: the defect reproduces deterministically in a unit test (see below), failing with the exact live shape (
delta from=2000while the consumer holds1000).Fix
Advance
lastPublishedBookVersiononly when a frame was actually encoded. An unsent version gap is fine: the next delta spans it (visible state → visible state), so the emitted stream is chain-continuous by construction. Genuine shed (bounded-queue drop, counted) still breaks the chain at the gateway — which is the correct signal, recovered by the #112 resnapshot.Audit finding fixed alongside (same family)
AppClusteredService.drainQueuegives up on a message for a session after 3 backpressuredsession.offerretries with no accounting anywhere — an egress loss invisible to every existing drop counter. Now counted asmatch_egress_offer_giveups_total(should stay 0) with a rate-limited CRITICAL log. Retry-policy hardening (esp. for the reliable OMS queue) left as a follow-up.Tests
MarketPublisherBookChainTest.deepBookChangeMustNotBreakTheEmittedVersionChain— snapshot → deep-only change (no frame, by design) → visible change → delta must chain from the last emitted frame's version. Fails pre-fix withexpected:<1000> but was:<2000>.MarketPublisherBookChainTest.emittedStreamIsChainContinuousAcrossManyMixedFlushes— chain continuity across 10 interleaved deep/visible flush cycles.🤖 Generated with Claude Code