Repository navigation
perf(dataplane): steady-state fast path at the command point (v5 S2b, default-off) - #316
Merged
Merged
Conversation
…lt-off) First real fast path: for a plain ClientCommand with the gate quiescent, the engine commits the Ready->Ready transition inline under the core lock and forwards directly, skipping the events->loop->cmd round-trip. Gated by TIPROXY_FSM_FASTPATH (default off), so with the flag unset the engine always takes the slow path and behaviour is unchanged. gate.rs: - FsmCore gains `fast_transitions`; `open` is now the single admission authority, read under the one lock by both admission paths. - `try_commit_steady(event, expected_effect, shutdown)`: under the lock, commits only when `open && !shutdown && pending == 0 && event == ClientCommand && FSM Ready` and a speculative clone yields exactly the single expected effect. On commit it writes the clone back, registers an in-flight GatePermit (so `pending` holds the floor across the forward, exactly as the slow path's effect permit does), bumps `fast_transitions`, and returns FastPath::Committed(permit). Otherwise Fallback with zero core writes. shutdown is read inside the lock (sender-closed counts as shutdown) so it linearizes with the commit. - `apply_slow(event, gated)` -> SlowOutcome: a gated ordinary event is DroppedClosed once the core is sealed (the slow-path shutdown-admission refusal, atomic with the transition); terminal events pass gated=false. session.rs: - The shutdown relay is now the loop's SOLE shutdown observer: it seals the core (open=false) under the lock BEFORE firing the loop oneshot, so "open sealed" and "loop drops the command" are one linearization point. The loop's direct has_changed precheck and mark_changed are removed. - `apply` takes `gated`: LoopAction::Event (engine ordinary events) is gated=true so a post-seal ClientCommand is dropped under the lock, matching the fast path refusing to commit; Control/Deadline/Probe and close_via_fsm's terminal events are gated=false. session_engine.rs: - Engine gains `core`, `shutdown`, and `fast_path_enabled` (read once from the env). The command point tries `try_commit_steady` for ClientCommand when enabled; Committed holds the in-flight permit across the forward (same scope as the slow path's await_effect permit), Fallback and every non-whitelisted event (incl. Quit) take the original slow path. Tests: gate-level commit + each fallback condition (shutdown value / sender-closed / sealed / pending!=0 / Quit / not-Ready) + effect-mismatch guard + the fast commit lands on the SAME FSM state as the slow transition (on/off FSM equivalence); gated apply dropped once sealed while terminal still applies; relay seals the core before firing on both a true signal and a dropped sender. All suites green with the flag off (behaviour-neutral); fmt + clippy --all-targets clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… loop open fence) CodexM5's S2b review found two windows where shutdown could be bypassed: 1. Relay startup race. `event_loop` entered its select immediately after spawning the relay, so a shutdown that already held (value `true` or a dropped sender) could be missed if an input event was already ready — the parent could dispatch before the relay sealed. shutdown_relay now makes its first observation up front (sealing the core if shutdown already holds, treating a closed sender as shutdown), acks via a new `started` oneshot, and only then fires `ServerShutdown`. `event_loop` awaits that ack before touching any input, restoring the synchronous precheck guarantee the removed direct `has_changed` read provided. 2. Seal->fire window. There was no loop-side `open` fence between the relay sealing the core and the loop observing the oneshot, and Deadline/BackendProbe/Control apply with `gated = false`, so they could still transition in that window. A post-select fence now returns `LoopEnd::Shutdown` whenever `!core.is_open()`, before any match/effect, dropping the dequeued action (and returning its permit). Only the relay seals `open` mid-loop, so `!open` there means shutdown; terminal close in `run` is still reached ungated. `Gate::is_open` is back to a non-test `pub(crate)` accessor (the loop fence reads it in production). Tests: relay unit tests now cover the startup ack and seal ordering for a pre-existing true signal, a pre-closed sender, and the ack-open-then-seal path on a later signal; a new loop test asserts that under a pre-existing shutdown a ready ClientCommand is never forwarded (no ForwardCommandToBackend / SendProxyGreeting). All suites green with the flag off; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… A/B Third S2b shutdown-adjacent gap (CodexM5): the backend probe snapshotted the FSM state before `backend_active().await` and applied `BackendIoError` afterwards without re-checking. A fast commit could turn `Ready -> Command` (holding a pending permit) during that await, so a stale `inactive` result would tear down an in-flight command. Fix — admit the probe atomically with the core: - gate.rs: `try_quiescent_operation(state_ok)` registers and returns an operation `GatePermit` only when the core is `open`, `pending == 0`, and `state_ok(state)` holds; otherwise `None`. - session.rs: the `BackendProbe` branch takes that permit (with `probe_safe`) and holds it across `backend_active().await`. While held `pending >= 1`, so `try_commit_steady` falls back and cannot commit a command mid-probe; a non-quiescent core simply skips the probe this round. The stale pre-await snapshot and its S2b TODO are gone. Wire-trace on/off A/B (the S2b acceptance item, CodexM5 chose this form): - tests/session_engine.rs `dump_fastpath_wire_trace`: drives a fixed steady-state scenario (SELECT/UPDATE x4) through the real engine + fake backend and prints the ordered command-phase trace (bytes forwarded to the backend + client outcomes) between markers, excluding the nondeterministic handshake. Reads `TIPROXY_FSM_FASTPATH` via the normal path. - scripts/fastpath-trace-ab.sh runs it twice in separate processes (flag unset vs `1`), strips the differing `fast_env` marker, and diffs. Verified locally: the two traces are byte-identical (fast path preserves the observable action sequence). Test: gate-level probe-permit coverage — not admitted when not probe-safe / sealed / `pending != 0`; admitted when quiescent, and while the permit is held `try_commit_steady` falls back (the probe-await x fast-commit interleaving) with the FSM untouched. All suites green with the flag off; `fmt --check` + `clippy --all-targets` clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bb7133
force-pushed
the
perf/fastpath-s2b
branch
from
October 5, 2026 14:14
4ea949e to
22a3274
Compare
…window) CodexM5's S2b review flagged two test-coverage gaps: 1. `preexisting_shutdown_refuses_a_ready_command` started from a fresh (Accept) session with the handshake queued first, so the pre-existing shutdown sealed the core before the first select and the first handshake event — not the ClientCommand — was what the fence dropped. Renamed to `preexisting_shutdown_before_handshake_admits_nothing` to describe what it actually proves (the startup-ack path admits nothing from Accept). 2. There was no deterministic test of the post-select `open` fence dropping a *ready* action in the window where the core is sealed but the shutdown oneshot has not fired. The relay/watch path can't hit that window deterministically from an external test, and `tests/` can't reach the `pub(crate)` core. New in-crate `session::fence_tests`: a minimal gated source + recording handler drive a `SessionLoop` over a core that was driven to `Ready` (`apply_slow`) and then `seal_closed()` — directly reproducing the sealed-but-oneshot-not-fired window. A gated `ClientCommand` is already in flight (its permit registered); the loop picks it in the first select and the fence refuses it before any match/effect. Asserts the command is never forwarded (zero `ForwardCommandToBackend`), the loop ends as `ServerShutdown`, and the dropped command's admission permit is returned (`pending` back to 0). Seal-before-fire ordering stays covered by the relay unit tests. All suites green with the flag off; fmt + clippy --all-targets clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bb7133
marked this pull request as ready for review
October 5, 2026 14:53
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.
Stacked on #315 (base
perf/fastpath-s2); the diff here is the single S2b commit. Will rebase/retarget ontomainonce #315 merges.First real fast path, gated by
TIPROXY_FSM_FASTPATH(default off). For a plainClientCommandwith the gate quiescent, the engine commits theReady→Readytransition inline under the core lock and forwards directly, skipping theevents→loop→cmdround-trip. With the flag unset the engine always takes the slow path — behaviour-neutral (all existing suites pass unchanged).Implements the design converged with @CodexM5 over three review rounds:
try_commit_steadyregisters aGatePermitat commit, held by the engine across the forward (same scope as the slow path'sawait_effectpermit) — preserves S1'spendingfloor.open=falseunder the core lock before firing the loop oneshot, so the engine's fast path (open) and the loop's gated apply (open) refuse at one instant. The loop's directhas_changedprecheck +mark_changedare removed.apply_slow(event, gated)drops a gated ordinary event once sealed (SlowOutcome::DroppedClosed), atomic with the transition; terminal events bypass (gated=false) soclose_via_fsmstill reachesClosed.open && !shutdown && pending==0 && event==ClientCommand && FSM Ready && clone.on_event==[ForwardCommandToBackend]; shutdown read inside the lock (sender-closed = shutdown); Fallback does zero core writes.Tests (deterministic): gate-level commit + every fallback condition + effect-mismatch guard + fast commit lands on the same FSM state as the slow transition (on/off FSM equivalence); gated apply dropped once sealed while terminal still applies; relay seals the core before firing (true signal + dropped sender).
Gates:
cargo test -p dataplaneall suites green (flag off);fmt --checkclean;clippy --all-targetsclean.Note on coverage: the full session-level on/off wire-action trace needs a real-Engine+backend harness that doesn't exist in-crate yet (the
tests/loop harness uses a generic source, not the engine). The gate/relay/loop tests cover the admission logic + seal linearization deterministically, and the post-permit forward code is shared by both paths. Proposing to @CodexM5: cover the end-to-end wire trace via a same-binary A/B at the integration/sysbench level (separate process runs, flag on/off), or build a minimal engine harness as a follow-up — whichever you prefer.🤖 Generated with Claude Code