fix(turn-driver): fence one Turn lane across hosts, not only one machine - #4605
huangruiteng wants to merge 1 commit into
Conversation
A Turn lane admits exactly one executing Turn, and the fence was a kernel lock held by the executing process. That is an authority inside one machine only: when the runtime root is shared -- an SSH-driven executor beside a local one -- the other host cannot see this lock, so two hosts could execute the same lane at once, each invoking its own host and spending its own slot. The same Turn now also holds a durable lane lease under the runtime root: an exclusive create naming this host (as a fingerprint), this process and this Turn, read before the kernel lock and refused while it is live. A lease whose holder is on another machine clears only by its own expiry, so a host that died mid-Turn cannot hold the lane forever; a lease left by a process on this machine clears as soon as that process is gone, because the kernel lock is what actually keeps two local Turns apart. A settled Turn releases its lease, and the typed turn_lane_in_flight refusal now names whichever holder refused -- this machine's lock or the other host's lease -- with the same all-false effect shape and the same pre-journal timing. Validation: tests/test_turn_lane_fence.py (8 tests) plus the 217-test turn driver set. The new cases cover a live lease from another host being refused before the journal, an expired lease and a crashed same-host holder not holding the lane, the holder's claim being readable by the next Turn, and the release leaving no claim behind. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head 2938a678445bcd9010f2e0ef8875acf9dcebfafe (re-read immediately before publication; unchanged). Policy revision 6. loopx pr-review --check-result returned ok: true, verdict APPROVE, for this exact head before publication.
动机
这条 PR 修的是 Turn lane 的单执行者契约在跨主机时失效：一条 lane 只允许一个 executing Turn，而这个门闩是执行进程持有的内核锁——它只在同一台机器内是权威。当 runtime root 被共享（例如 SSH 驱动的执行者与本地执行者并存），另一台机器看不到这把锁，于是两台 host 可以同时执行同一条 lane：各自调用自己的 host、各自写自己的 delivery、各自花自己的 quota slot，一条有界问题得到两个答案。
- 影响面：凡是共享 runtime root 上的 executing Turn。
- 之后的代价：journal / delivery / quota 三者对同一条 lane 的说法互相矛盾，只能人工对账。
- 判定：
justified_increment——把门闩从"本机内核"扩展到"共享根上的一个持久事实"。
改动思路
让门闩由两个事实组成，各管各的边界：
- 内核锁继续管本机（它能让崩溃/被杀掉的 Turn 立刻释放 lane，不留陈旧占用）。
- 新增持久 lane lease放在 runtime root 里：用
O_CREAT|O_EXCL独占创建，记录本机指纹、pid、此次 Turn 身份与到期时间；在读内核锁之前先读它，活着的 lease 直接拒绝。 - 过期是唯一能清掉"死在半路的远程 host"的方式（本机无法观察别的机器的进程）；本机留下的 lease 只要那个进程不在了就立即接管（本机本来就有内核锁兜底）。
- Turn 结束时释放自己的 lease（只在确认是自己的记录时才删），所以正常流程不会给后来的 Turn 或 host 留任何占用。
具体改动
2 个文件、+377/-12：production loopx/control_plane/turn_driver/lane_fence.py +234/-8（lease 形状、存活判定、claim/release、门闩接线、拒绝时优先读 lease holder）；测试 +143/-4。
关键代码讲解
loopx/control_plane/turn_driver/lane_fence.pyturn_lane_singleflight：先读持久 lease（拒绝得早），再取内核锁，取锁之后再 claim 一次（关掉两次读之间的竞态窗口），退出时释放；claim 失败则拒绝并让内核锁随上下文释放。- 同文件
_turn_lane_lease_is_live：没有可解析到期时间的记录不算占用；异机记录活到到期为止；同机记录只看 pid 是否还在，且 pid 存活"无法判定"时按占用处理（fail-closed，宁可等也不重复执行）。 - 同文件
_claim_turn_lane_lease：O_CREAT|O_EXCL就是 claim——两台 host 同时发现 lane 空闲也不可能都成功；只有在既有记录已不占用时才原子替换（接管）。任何OSError都拒绝，而不是无闩执行。 - 同文件「拒绝时报出的 holder」：优先读持久 lease 的 public-safe 投影（只有 agent_id / operation / acquired_at / pid），再回落到内核锁的持有者——所以"另一台机器在跑"的拒绝也能说清在等谁，同时不泄露主机名与本地路径（测试断言了这两点）。
语义与 CI 对齐
semantic_alignment：aligned / reuse_existing。改动只改执行分支的准入判断，拒绝载荷仍走原来那一份（turn_lane_in_flight + wait_for_in_flight_turn + all-false effects + host: not_invoked），没有第二条准入路径、没有新的拒绝形状。原本想复用 loopx/control_plane/work_items/task_lease.py，但它本质是 Todo handoff/coordination 权威（有 handoff mode、lease epoch），让 lane 借用它会把 lane 伪装成 Todo；所以这里只复用"原子写"的做法，语义留在 lane fence 自己的边界里。本地证据：8 条 lane-fence 测试、217 条 turn-driver 测试、loopx canary premerge --from-git-diff passed 0 failure。
对主干的风险
- 爆炸半径：只有 executing Turn 的准入；preview 分支不取门闩、行为不变，本机拒绝的载荷形状逐字段不变（原测试仍通过）。
- 最大的一处权衡：没有 renewal 时，死在半路的远程 host 会占住 lane 到 30 分钟 TTL 到期。这是"极少数 stall vs 静默重复执行已提交工作与 quota"的取舍——我选了前者，因为 stall 是有界的、并且以具名 holder 的 typed 拒绝暴露出来，而重复执行是静默的。后继切片正是 renewal（活着就续期，从而把 TTL 缩短）+ 运营可达的"释放该 lane"，已记在
todo_63a2f1d17c43。 - 未验证面：没有真实的多主机运行，也没有网络文件系统；
O_EXCL的跨主机原子性依据的是共享文件系统的既有语义，而不是我实测出来的。这一点我写进了 review 的 residual risk，没有假装已经验证。 - 旧数据/兼容：没有 lease 文件时行为与改动前完全一致；旧 runtime 只会忽略这个文件。
我的整体评价
同意合并（待 owner 决定；本 PR 属控制面 runtime 行为面，按现行规则只提 PR、不自合并、不 admin-bypass）。
这是一个正向、且 proportional 的切片：它补上的是 F4 措辞里"exactly once, behind an execution barrier"里真正跨主机的那一半——用一个持久事实（独占创建 + 异机以到期为界 + 同机以进程存活为界）把"单执行者"从"单机器"扩展到"共享 runtime root"。机制成本是一个记录形状、一条存活规则、一对 claim/release，全部长在既有门闩里；拒绝仍是原来那一份 typed 载荷，且明确不泄露主机名与本地路径。它同时诚实标出了自己的代价与未经实测的部分（TTL stall、跨主机原子性），并把缩短窗口的 renewal 交给后继切片，而不是把风险留在注释里。
English verdict: APPROVE - exact head 2938a678445bcd9010f2e0ef8875acf9dcebfafe of #4605 makes the Turn lane's single-executor fence hold across hosts that share the runtime root: the executing Turn now also claims a durable lane lease (exclusive create naming a host fingerprint, pid, Turn and expiry), a live lease from another machine refuses the second Turn with the shipped typed turn_lane_in_flight packet before the journal, and expiry or a dead local process releases the lane, so a real Turn always releases its own claim. The refusal names whichever holder refused without publishing the host name or any path, and the local refusal, preview path and payload shape are unchanged. Validation: 8 lane-fence tests, 217 turn-driver tests, canary premerge passed with 0 failures. Residual risk, disclosed: a remote host that dies mid-Turn holds its lane until the 30-minute TTL (renewal and an operator release path are the successor slice on todo_63a2f1d17c43), and cross-host atomicity of the exclusive create rests on the shared filesystem's semantics rather than a measured multi-host run. Control-plane change: proposed for review only, no self-merge and no admin bypass.
|
Direction assessment at Reproduced stale-takeover race: _claim_turn_lane_lease uses O_EXCL only for an absent file. When a file exists but looks expired, it reads the record and unconditionally replaces it. In an isolated deterministic interleaving using the exact-head production functions: A reads the expired record; B claims and writes its live record; A resumes with its stale read and replaces B's record. Both calls return True. A crash/expiry recovery therefore admits two holders under the very assumption that kernel locks are not shared across hosts. Atomic replacement is not conditional ownership transfer. The empty-file interval between exclusive creation and JSON publication also needs a fail-closed rule. Expiry is also not fencing: the lease is never renewed, and expiry makes it free even when the original Turn is still executing. A duration assumption is insufficient unless the host enforces it and prevents late effects; renewal must be coupled to stale-holder rejection at the relevant effect/commit boundary. Please implement claim/takeover/release with conditional identity/version checks under the appropriate authority, qualify concurrent takeover and late old-holder effects, then run the actual two-host/shared-store path. No real shared-filesystem qualification was performed in this assessment. Cross-RFC scope: this is runtime/session exclusion. It does not close R1's team-plan execution barrier, which must also prevent dependent lanes from running against incomplete materialization. Keep those two contracts distinct. Do not add an independent distributed lease authority merely to work around a missing integration with the selected authority/runtime profile. |
|
Closing under maintainer direction to consolidate the steward architecture. The reproduced stale-lease takeover race and lack of stale-executor fencing mean this implementation must not ship as cross-host single execution. The existing main implementation remains in place. A future cross-host slice must use the selected authority/runtime ownership and real two-host qualification; it is separate from the R1 team-plan commit barrier. The branch and discussion are retained as design evidence. |
1 participant
Control-plane change — author hands this over unmerged. Per the owner's direction, this is proposed and reviewed on its exact head, then left for the maintainer to merge. No author self-merge, no admin bypass.
Goal And Delivered Outcome
docs/reference/protocols/loopx-turn-v0.md). A lane admits exactly one executing Turn, and the fence was a kernel lock held by the executing process — an authority inside one machine only. When the runtime root is shared (an SSH-driven executor beside a local one), the other host cannot see that lock, so two hosts could execute the same lane at once: two hosts, two deliveries, two quota slots for one bounded question.run_loopx_turn_once(execute=True)on the second host would proceed to require its writeback/spend/scheduler callbacks and execute the Turn. After, the same call returns the typed refusal (reason: turn_lane_in_flight,remediation: [wait_for_in_flight_turn], all-false effects,quota_slot_spend_count: 0) and names the holder, before the journal, the host and quota. The failing-before check is the new foreign-host case: on the previous runtime there was no durable fact, so the refusal could not be produced at all.todo_63a2f1d17c43(whichunblocks_todo_id: todo_5894bd5f8027); basemainate66615d33.Scope And Continuation
30 * 60) is the only way a lease left by a host that died can clear, so a crashed remote host holds its lane for that window; the intended replacement is a renewal heartbeat from the executing Turn (renew while alive, so the TTL can be much shorter) plus an operator-reachable "release this lane" path. That is a separate slice: it changes the renewal cadence of the same lease and needs its own failure walkthrough, and the shipped behavior is strictly safer than before without it. Recorded on the Todo. The cross-host task lease (loopx/control_plane/work_items/task_lease.py) was reviewed as the alternative owner and not used: it is a Todo handoff/coordination authority with handoff modes and epochs, and a Turn lane is neither a Todo nor a coordination decision, so the lane lease reuses only the atomic-write discipline.Validation
pytest tests/test_turn_lane_fence.py— 8 passed. New cases: a live lease from another host is refused before the journal with the all-false effect shape and a public-safe holder readback; an expired foreign lease does not hold the lane and is replaced by this Turn's own claim; a lease left by a crashed process on this machine does not hold the lane; the holder's claim is the fact the next Turn reads; release leaves no claim behind and the lane is immediately executable again.pytestover the turn driver set (test_turn_lane_fence,test_loopx_turn_driver,test_loopx_turn_executor,test_loopx_turn_host_failure,test_loopx_turn_managed_step,test_loopx_turn_settlement_parity,test_loopx_turn_transaction,test_turn_envelope) — 217 passed, so the existing local refusal, the preview path and the settled-Turn release are unchanged.loopx canary premerge --from-git-diff— passed, 0 failures (control_plane + python surfaces, including the maintainability ratchet and the vocabulary drift smoke).Boundary
Control-plane runtime (
loopx/control_plane/turn_driver/lane_fence.py) plus its test. No CLI, API, frontend or Lark change. Proposed for review; not self-merged.