Fix: enforce single root Remote L3 startup deadline; split attach vs runtime timeout#1432
Merged
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughRemote L3 startup now uses a shared absolute deadline across Python session setup and activation. The C++ transport separates attach and runtime timeouts, while bindings, worker APIs, and tests are updated for the new contracts. ChangesRemote L3 deadline budgeting
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Worker
participant RemoteRunner
participant RemoteSession
participant RemoteL3SocketTransport
Worker->>RemoteRunner: start with remaining startup budget
RemoteRunner->>RemoteSession: propagate shared startup deadline
Worker->>RemoteSession: open session using absolute deadline
RemoteSession->>RemoteL3SocketTransport: attach with remaining attach timeout
RemoteL3SocketTransport-->>Worker: attach result
Worker->>Worker: recheck deadline before READY
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ChaoWao
force-pushed
the
feat/p0.3-deadline-hardening
branch
6 times, most recently
from
July 22, 2026 07:03
9723ee5 to
1370faa
Compare
…plit attach vs runtime timeout hw-native-sys#1420 delivered the shape of a single-root remote-L3 startup budget but left the budget resettable at several hops, and clamped the runtime command timeout to leftover startup budget. R2 [high] — make the single root deadline a hard wall across the whole parent -> daemon -> runner -> native-attach chain. R4 [medium] — separate the attach timeout from the post-READY runtime command timeout. The cross-machine parent -> daemon wire carries only a duration; no cross-machine wire fields are added. Parent (worker.py): - Remote endpoints are numeric-only (or localhost), validated in add_remote_worker before any startup resource: getaddrinfo resolution is unbounded and uncancellable, so a hostname is rejected up front rather than mid-activation (which would roll back the whole forked tree). RemoteWorkerSpec and the design doc state the contract. - _open_remote_session(deadline): AI_NUMERICHOST resolution cannot block; every per-address connect and each recv derive from the deadline; the wire budget and send timeout are derived after the manifest build. - The final root-deadline gate lives in the READY-commit critical section and applies to every hierarchical worker (a local child may have remote descendants), so a thread descheduled after startup cannot publish READY late. Remote (remote_l3_worker.py, remote_l3_session.py): - The daemon builds one absolute deadline up front and, over the private same-host daemon -> runner manifest, hands the runner its ABSOLUTE monotonic deadline (shared CLOCK_MONOTONIC). The runner uses it verbatim, so its deadline equals the daemon's and spawn/import/registry time is charged; it establishes the deadline before registry install. - Waiting for the parent to attach is still the attach phase: command accept is bounded by the remaining startup budget, not session_timeout_s. The command connection then stays blocking: the command loop idle-waits for the next frame (forced blocking, since accept() inherits the process default timeout), so a finite timeout would tear down a healthy idle session; the loop ends when the parent closes the command socket (EOF). C++ (remote_endpoint.{h,cpp}, worker.{h,cpp}, worker_bind.h): - RemoteL3SocketTransport takes attach_timeout_s + runtime_timeout_s and one attach_deadline_. connect_tcp_socket takes the absolute Deadline directly (resolution + each connect derive from it). Command/HELLO/ health connect use attach_deadline_; runtime frame I/O + the health loop use runtime_timeout_s_. The connected fd stays O_NONBLOCK for its whole life and all frame I/O polls for readiness under a deadline, so a large write to a stalled reader cannot outlast the deadline in one blocking send() (the earlier per-send MSG_DONTWAIT was not portable and hung ~47s on macOS); recv/read paths handle EAGAIN. add_remote_l3_socket and its binding gain both timeouts (internal C++ ABI; not wire). Tests are repro-first (discipline §3): deterministic fake clock; the single-root-budget suite on the deadline API; attach-vs-runtime split; READY-commit-gate (unconditional for hierarchical startup); numeric-only endpoint contract (hostname rejected at registration); command-accept bounded by the startup deadline (distinct from session_timeout); daemon-hands-runner-absolute-deadline and runner-uses-it-verbatim regressions; and cpput tests proving the HELLO read is bounded by the attach timeout, a runtime read survives an elapsed attach deadline, and a runtime write to a stalled reader times out at the runtime deadline.
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.
Summary
#1420delivered the shape of a single-root Remote L3 startup budget but left it resettable at several hops and clamped the runtime command timeout to leftover startup budget (KNOWN_ISSUES.md/ P0.3 roadmap).parent → daemon → runner → native-attachchain.Wire contract: the cross-machine
parent → daemonwire carries only a duration (startup_remaining_s) — no absolute monotonic timestamp crosses machines. The same-hostdaemon → runnerhandoff is a private local manifest (written to a tempfile the daemon Popen-spawns the runner with), and it additionally carriesstartup_deadline_monotonic— the daemon's absolute deadline on the sharedCLOCK_MONOTONIC, so the runner's deadline equals the daemon's and spawn/import/registry time is charged (not restarted from a frozen duration).Changes
Parent (
worker.py)localhost), validated inadd_remote_workerbefore any startup resource:getaddrinfois unbounded/uncancellable, so a hostname is rejected up front rather than mid-activation (which would roll back the forked tree).RemoteWorkerSpec+docs/remote-l3-worker-design.mdstate the contract._open_remote_session(deadline):AI_NUMERICHOSTresolution can't block; each per-address connect and each recv derive from the deadline; wire budget + send timeout derived after the manifest build.Remote (
remote_l3_worker.py,remote_l3_session.py)startup_deadline_monotonic; runner uses it verbatim, established before registry install. Command accept (still attach phase) is bounded by the remaining startup budget, notsession_timeout_s. The command connection is then forced blocking (settimeout(None), sinceaccept()inheritssocket.getdefaulttimeout()which a user module could have set) — the command loop idle-waits for the next frame, so a finite timeout would self-destruct a healthy idle session; the loop ends when the parent closes the command socket (read_framesees EOF).C++ (
remote_endpoint.{h,cpp},worker.{h,cpp},worker_bind.h)RemoteL3SocketTransport(attach_timeout_s, runtime_timeout_s), oneattach_deadline_;connect_tcp_sockettakes the absoluteDeadlinedirectly. Command/HELLO/health connect useattach_deadline_; runtime frame I/O + health loop useruntime_timeout_s_. The connected fd staysO_NONBLOCKfor its whole life and all frame I/O polls for readiness under a deadline, so a large write to a stalled reader cannot outlast the deadline in one blockingsend()(the earlier per-sendMSG_DONTWAITwas not portable — hung ~47s on macOS); recv paths handleEAGAIN.add_remote_l3_socket+ binding gain both timeouts (internal C++ ABI, not wire).Testing (repro-first, device-free)
session_timeout); daemon-hands-runner-absolute-deadline + runner-uses-it-verbatim; bounded-connect; send-time recompute. Behavioral tests fail against the pre-fix sources.Local verification: cpput
test_remote_endpoint13/13 (Linux);tests/ut/py/test_worker/+ remote lifecycle/protocol + callable_identity 397 passed / 2 skipped; ruff + ruff-format + pyright + clang-format + clang-tidy + cpplint + markdownlint all pass. macOS runtime-write regression is fixed by theO_NONBLOCKchange (CI will confirm). Hardware N/A (sim-only control plane).The roadmap entry for hierarchical remote startup should stay PARTIAL until the test-level-hard-timeout + full "local child READY then remote fails" zero-residual (PID/SHM/session) matrix exists — out of scope here.