Skip to content

[TENT] Half-Open rail state machine + admit/isAvailable split - #1

Closed
Colors-111 wants to merge 2 commits into
rate-limit-logfrom
half-open-probes
Closed

Colors-111 wants to merge 2 commits into
rate-limit-logfrom
half-open-probes

Conversation

@Colors-111

@Colors-111 Colors-111 commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Description

Half-Open rail state machine + admit/isAvailable split

Follow-up to PR kvcache-ai#3946 (defect A + log rate-limiting). kvcache-ai#3946 capped the burst-escalation defect (cooldown 30s vs 300s) and rate-limited the failure-storm logs. A reviewer asked to split the planned Half-Open / Open-phase probing into this follow-up with one prerequisite:

available() is a mutating admit used as a predicate (called across updateBestMapping/fallback/GDR), and with default probe_interval=1s there is no in-flight tracking, so a second probe can go out while the first is still on the wire. The new tests use probe_interval=60, so they don't catch that. Happy to review Half-Open once admit vs query is split and in-flight is tracked.

This PR does exactly that: splits available() into a pure predicate isAvailable() + a mutating admit() with per-rail in-flight tracking, and adds Open-phase probing + Half-Open.

State machine: Closed → Open → Half-Open → Closed/Open

  • Closed (!paused(), half_open=false, probe_in_flight=false): healthy. isAvailable()=true, admit()=true (no mutation). Normal transfers; completion is the fast-path markRecovered no-op.
  • Open (paused(), now < resume_time, half_open=false): isAvailable()=false. admit() arms ONE exploratory probe per probe_interval_ (sets probe_in_flight + last_probe_time). Probe success → markRecovered → Closed (defect B: transient faults recover in seconds, not the full cooldown). Probe failure → markFailed no-op (was paused, defect A), flag cleared.
  • expired-Open (paused(), now >= resume_time, half_open=false): isAvailable()=false. admit() transitions to Half-Open and arms one trial. If an Open probe is still in flight at expiry, admit() returns false until it resolves.
  • Half-Open (paused(), half_open=true, probe_in_flight=true): isAvailable()=false, admit()=false (one trial only). Trial success → markRecovered → Closed, backoff reset to 0. Trial failure → markFailed escalates cooldown *= 2 (cap 300s) and re-arms → Open.

resume_time stays armed through Half-Open so paused() stays true (not mistaken for Closed and flooded). Escalation happens only on a Half-Open trial failure; clock expiry alone never escalates. This removes the "expiry fully reopens" path that slammed a still-dead peer with every slice and re-triggered the storm at 30s/60s/120s.

The split

available() had two jobs conflated: a query (is this rail usable?) and an admit (route a transfer through it, possibly arming a probe). It was called from:

  • updateBestMapping (rail_monitor.cpp) — pure query, but the old expiry branch mutated state AND called updateBestMapping, recursing.
  • selectOptimalDevice / selectFallbackDevice (workers.cpp) — admit; on true the slice is posted.

Split:

  • bool isAvailable(int, int) const — pure predicate (!paused()). No mutation, no updateBestMapping → recursion gone. Used by updateBestMapping and any query.
  • bool admit(int, int) — the only mutating admit. Closed → true (no-op). Open → one probe per probe_interval_ (in-flight gated). expired-Open → Half-Open trial. Half-Open → false (trial in flight).
  • void cancelProbe(int, int) — clears an armed probe/trial when a slice selected the rail but never reached the wire (see below).

In-flight tracking (the reviewer's bug)

admit() arms probe_in_flight at selection time, but selection ≠ posting. If a slice arms a probe then never reaches the wire, the flag is orphaned and the rail can never probe again. Three paths cause this; all are handled:

  1. GDR-exclusion after admit (selectFallbackDevice): reordered the GDR check before admit, so a probe is never armed on a GDR-excluded pair. (selectOptimalDevice already short-circuits GDR before admit.)
  2. getEndpoint-null re-queue (asyncPostSend): no endpoint obtained → cancelProbe before submitFromTick.
  3. HW-reject re-queue (asyncPostSend): submitSlices rejected the slice pre-wire → cancelProbe before submitFromTick.

cancelProbe clears probe_in_flight and reverts half_open to expired-Open so the next admit re-arms. It is a no-op when no probe is in flight (normal transfers), so callers invoke it unconditionally on the re-queue path. Single-threaded per-worker ownership (each WorkerContext::rails[machine_id] is one RailMonitor on one worker thread) makes a bool sufficient: at most one probe/trial per rail, and the completion path (markFailed/markRecovered) clears the same flag the slice armed.

What stays out of scope

  • Store-kill: a probe still hits the dead old port until master drops the old segment (~51s). That is master-side unmount_expired_mem_segment / stable segment names, not RailMonitor backoff. Half-Open only stops expiry from re-flooding the dead port; once a fresh replica is reachable, a trial succeeds within one probe_interval_.

Verification

  • Unit tests (tent/tests/rail_monitor_test.cpp):
    • BurstFailuresDoNotEscalateCooldown (probing disabled): 8× markFailed; admit()=true at 1.5s (cooldown stayed 1s → trial), false if escalated to 256s.
    • TrialFailureEscalatesCooldown (probing disabled): trial fail → 1→2s, re-arm; 2s cooldown expires → trial.
    • TrialSuccessResetsBackoff (probing disabled): trial success → Closed; next pause uses 1s, not 2s.
    • ExpiryAdmitsOneTrialNotFullReopen (probing disabled): expiry admit()=true (trial), next admit()=false (in-flight).
    • InFlightProbeBlocksSecondAdmit (default probe_interval=1s): admit()=true (probe armed), admit()=false (in-flight), markRecovered clears → new probe arms. This is the test the reviewer said was missing.
    • CancelProbeRevertsArmedTrial: trial armed → cancelProbe → re-admits a fresh trial; no-op on a Closed rail.
    • 10 baseline tests renamed available()isAvailable() (query semantics unchanged); CooldownDoesNotCarryOverAfterRecovery adapted (final expiry assertion → admit() with probing disabled).
  • Probing-disabled vs default: burst/escalation tests disable probing so a cooling rail's admit()=false while an expired rail's admit()=true — this discriminates cooldown duration. InFlightProbeBlocksSecondAdmit uses the default 1s to exercise in-flight directly (not to hide it).
  • End-to-end: cooldown capped at 30s (defect A); expiry no longer floods; one probe/trial per probe_interval tests recovery.

Files Changed

  • tent/include/tent/transport/rdma/rail_monitor.hRailState gains last_probe_time, half_open, probe_in_flight; available()isAvailable()/admit()/cancelProbe(); new probe_interval_ + kCfgProbeIntervalSecs.
  • tent/src/transport/rdma/rail_monitor.cppisAvailable/admit/cancelProbe; markFailed/markRecovered in-flight + Half-Open handling; updateBestMappingisAvailable; load() reads the probe key.
  • tent/src/transport/rdma/workers.cppselectOptimalDevice/selectFallbackDevice available()admit(); GDR check moved before admit in fallback; cancelProbe on the two pre-wire re-queue paths.
  • tent/tests/rail_monitor_test.cpp — 10 renames, 2 adapted, 4 new tests.

Configuration

Key Default Description
transports/rdma/rail_error_threshold 3 error count threshold in the window
transports/rdma/rail_error_window_secs 10 error-count window
transports/rdma/rail_cooldown_secs 30 initial cooldown (seconds)
transports/rdma/rail_probe_interval_secs 1 probe/trial interval while paused; 0 disables probing

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

# Example: bash scripts/run_ci_test.sh

Test results:

  • Unit tests pass
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

@Colors-111 Colors-111 closed this Sep 15, 2026
Repository owner locked and limited conversation to collaborators Sep 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant