perf(egress): take the OMS streams off the market-data conflation timer - #159
Merged
Conversation
Order lifecycle was 16.4 ms at the client, and none of it was work. It was two
timers in series.
MarketPublisher buffers everything a market produces and flushes on a fixed
20 ms ScheduledExecutorService. That timer exists for CONFLATION, and for two of
its four outputs conflation is exactly right: the aggregated trade feed collapses
many fills at a price level into one record, and the book snapshot is a
20-level picture that only the latest version of matters.
The other two outputs are not conflatable at all. TradeExecutionBatch and
OrderStatusBatch are per-order FACTS bound for the OMS: every one has to arrive,
and a dropped terminal leaves an OMS hold stuck (oms#21). They were riding the
conflation timer anyway, so an order status waited a uniform 0-20 ms for a tick
that had nothing to do with it.
They now go out on every Disruptor endOfBatch instead. That signal was already
in the handler signature and is the honest form of opportunistic batching: busy
means the Disruptor hands over large batches and large batches go out, idle means
a batch of one and the latency is the offer itself. The 20 ms scheduler still
calls the same code as the idle-path backstop for a trailing partial batch, and
still owns the market-data half.
Measured, docker multi-host, 20k orders/s, same run shape, the new me-leg
histogram (ME order offer -> first status, with the hold leg and any schedule
backlog removed):
p50 p90 p99
before 16.10 ms 25.05 ms 29.31 ms
after 5.46 ms 9.50 ms 10.63 ms
10.64 ms off p50 from this one change. The residual 5.46 ms is the OTHER timer,
AppClusteredService's 10 ms ACTIVE_FLUSH_INTERVAL_MS gate — its p90 of 9.50 ms
is that gate's full period, which is what a wall-clock gate looks like from the
outside. Handled separately.
Why we know it was a timer and not queueing: the lifecycle sat at a flat 11.2,
11.2 and 11.9 ms at 5k, 20k and 80k orders/s. A 16x load change moved it 0.7 ms.
Queueing does not do that; a fixed periodic wait does, and the p50/p90 of
11/19 ms is a uniform wait on a ~20 ms period. Meanwhile the assets cluster,
same box, same run, same generator thread, answered fund locks in 261 us — so
neither the harness nor consensus was the cost.
Thread safety is unchanged by construction: flushBuffers and the buffer methods
already share this object's monitor, so the new call site takes the same lock and
the shared encodeBuffer stays safe across the Disruptor thread and the scheduler.
Determinism is not affected and the corpus is not the gate here: MarketPublisher's
scheduler is a plain ScheduledExecutorService on a Disruptor thread, not a
replicated cluster timer, so no log entries and no log shape change. The 85
determinism/snapshot tests pass regardless.
Trade and order counts identical across the A/B (457,691 trades both sides),
zero settle faults.
Refs task #12, ME side.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second of the two timers. With the OMS batches now assembled upstream on the
Disruptor's endOfBatch, the service thread's wall-clock gate was pure latency:
every client-visible order status waited a uniform 0-10ms for a tick that no
longer bought anything.
The gate existed to bound flush overhead when MarketPublisher was handing this
queue a fresh batch only every 20ms. Now a batch arrives already batched, so
draining per message moves bytes, it does not shrink batches. Draining
unconditionally is also the right shape for the constraint: ClientSession.offer
is only legal from a log-driven callback, so the end of onSessionMessage is the
ONLY place a leader can push egress at the rate work actually arrives.
Removed rather than set to zero, so the per-message System.currentTimeMillis()
goes with it.
Measured, docker multi-host, 20k orders/s, ME leg (order offer -> first status):
p50 p90 p99
both timers 16.10 ms 25.05 ms 29.31 ms
conflation timer only 5.46 ms 9.50 ms 10.63 ms
neither 495 us 678 us 1.20 ms
Client-visible order lifecycle over the same three: 16.38 ms -> 5.73 ms -> 757 us.
The matching engine's leg is now 495 us against the assets cluster's 260 us for a
fund lock on the same box in the same run. Both are quorum-committed round trips,
and matching does more work than moving a balance, so that ratio is finally what
the architecture predicts. Nothing left in there is a timer.
The 250ms MARKET_DATA_FLUSH_INTERVAL_MS cluster timer stays exactly as it is: it
is the idle-market backstop and the match#25/#26 liveness chain, it is a
replicated timer, and it is now never on the active path.
Trade and order counts identical across all three builds (457,691 trades), zero
settle faults. 85 determinism/snapshot tests pass.
Refs task #12, ME side.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI caught that the MarketPublisher tests encoded the old behaviour, and it was right to. Every test drove events with endOfBatch=true, which was a harmless shorthand for "deliver this event" back when the flag did nothing. Now it means "this batch is complete, send it", so three statuses produced three frames where the test asserted one. The tests wanted to assert coalescing, and coalescing is still exactly what happens under load — the Disruptor marks every event of a batch except the last with endOfBatch=false. The tests were simply not modelling a batch. They now deliver mid-batch events with false and let the explicit flush close the window, which is both what they meant and what production does. Added the contract the change actually introduces, so it is asserted on purpose rather than surviving by accident: a batch stays buffered mid-batch, goes out at endOfBatch with no flush call, and arrives as ONE frame carrying the whole batch rather than one frame per event. 491 tests green. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Order lifecycle was 16.4 ms at the client, and none of it was work. It was two timers in series.
MarketPublisherbuffers everything a market produces and flushes on a fixed 20 msScheduledExecutorService. That timer exists for conflation, and for two of its four outputs conflation is exactly right: the aggregated trade feed collapses many fills at a price level into one record, and the book snapshot is a 20-level picture where only the latest version matters.The other two outputs are not conflatable at all.
TradeExecutionBatchandOrderStatusBatchare per-order facts bound for the OMS: every one has to arrive, and a dropped terminal leaves an OMS hold stuck (oms#21). They were riding the conflation timer anyway, so an order status waited a uniform 0-20 ms for a tick that had nothing to do with it.They now go out on every Disruptor
endOfBatch. That signal was already in the handler signature and is the honest form of opportunistic batching: busy means large batches in and large batches out, idle means a batch of one and the latency is the offer itself. The scheduler still calls the same code as the idle-path backstop for a trailing partial batch, and still owns the market-data half.Measured
Docker multi-host, 20k orders/s, same run shape. The new
me-leghistogram isolates ME order offer → first status, with the hold leg and any schedule backlog removed:10.64 ms off p50 from this one change.
The residual 5.46 ms is the other timer —
AppClusteredService's 10 msACTIVE_FLUSH_INTERVAL_MSgate. Its p90 of 9.50 ms is that gate's full period, which is what a wall-clock gate looks like from outside. Handled separately.How we know it was a timer and not queueing
The lifecycle sat at a flat 11.2, 11.2 and 11.9 ms at 5k, 20k and 80k orders/s. A 16x load change moved it 0.7 ms. Queueing does not do that. A fixed periodic wait does, and a p50/p90 of 11/19 ms is a uniform wait on a ~20 ms period.
Meanwhile the assets cluster — same box, same run, same generator thread — answered fund locks in 261 µs. So neither the harness nor consensus was the cost.
Safety
flushBuffersand the buffer methods already share this object's monitor, so the new call site takes the same lock and the sharedencodeBufferstays safe across the Disruptor thread and the scheduler.MarketPublisher's scheduler is a plainScheduledExecutorServiceon a Disruptor thread, not a replicated cluster timer. No log entries, no log shape change. The 85 determinism/snapshot tests pass regardless.Refs task #12, ME side.
🤖 Generated with Claude Code