Skip to content

Commit 2caada6

Browse files
authored
Merge pull request #4812 from loopx-project/codex/turn-lane-holder-host-20260920
fix(turn-lane): name the holding machine beside the holding pid
2 parents a5d678d + 2437d3e commit 2caada6

4 files changed

Lines changed: 40 additions & 7 deletions

File tree

‎loopx/control_plane/turn_driver/lane_fence.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@
3333
TURN_LANE_DIR_NAME = ".lanes"
3434
TURN_LANE_UNATTRIBUTED_AGENT = "unattributed"
3535
# Public-safe holder fields only: the lock record also carries a lock id, a
36-
# policy name, and the private lock path, which never leave this process.
37-
TURN_LANE_HOLDER_TEXT_FIELDS = ("agent_id", "operation", "acquired_at")
36+
# policy name, and the private lock path, which never leave this process. The
37+
# host is projected because two hosts can share one runtime root: a refusal on
38+
# the second host must not print a pid that cannot exist there.
39+
TURN_LANE_HOLDER_TEXT_FIELDS = ("agent_id", "operation", "acquired_at", "host")
3840
# A refusal taken here stops before the journal, the host, and quota, so the
3941
# payload reports the same effect shape an executing Turn does -- all false.
4042
TURN_LANE_NO_EFFECTS: dict[str, bool] = {
@@ -104,9 +106,11 @@ def turn_lane_singleflight(
104106
def turn_lane_holder_readback(target: Path) -> dict[str, Any]:
105107
"""Return the public-safe identity of the Turn holding one lane, else ``{}``.
106108
107-
Only names, a timestamp, and a process id are projected: the holder record's
108-
private lock path and lock id stay out, so a refusal can say who is running
109-
without publishing where this machine keeps its runtime state.
109+
Only names, a timestamp, a machine name and a process id are projected: the
110+
holder record's private lock path and lock id stay out, so a refusal can say
111+
who is running where without publishing where a machine keeps its runtime
112+
state. The machine name is what makes the projected pid actionable when two
113+
hosts share one runtime root.
110114
"""
111115

112116
try:

‎loopx/file_lock.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import os
1111
from pathlib import Path
1212
import re
13+
import socket
1314
import tempfile
1415
import time
1516
import importlib
@@ -147,6 +148,11 @@ def _identity(
147148
) -> dict[str, object]:
148149
return {
149150
"pid": os.getpid(),
151+
# A pid is only meaningful on the machine that wrote this record. Two
152+
# hosts sharing one runtime root can both read the holder, so the record
153+
# names its own machine and a reader never has to guess which host a pid
154+
# belongs to. The name is a sanitized label, not a path or a secret.
155+
"host": _safe_label(socket.gethostname(), fallback="unknown"),
150156
"agent_id": _safe_label(
151157
agent_id or os.environ.get("LOOPX_AGENT_ID"),
152158
fallback="unknown",
@@ -250,6 +256,7 @@ def _read_holder_record(lock_path: Path) -> dict[str, object]:
250256
"schema_version",
251257
"lock_id",
252258
"policy",
259+
"host",
253260
"pid",
254261
"agent_id",
255262
"operation",
@@ -263,10 +270,13 @@ def _operator_action(holder: dict[str, object], *, retry_mode: str) -> dict[str,
263270
return {
264271
"required": True,
265272
"action": "inspect_lock_holder",
273+
# The pid is only meaningful on this host: naming it stops an operator
274+
# from hunting for a process id that cannot exist on another machine.
275+
"holder_host": holder.get("host"),
266276
"holder_pid": holder.get("pid"),
267277
"retry_mode": retry_mode,
268278
"steps": [
269-
"Inspect the recorded holder PID and operation.",
279+
"Inspect the recorded holder host, PID and operation on that host.",
270280
"Confirm the process is stalled before terminating it.",
271281
"Retry according to retry_mode after the holder exits.",
272282
"Do not delete the lock file; the kernel lock is authoritative.",

‎tests/test_file_lock.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
import os
55
from pathlib import Path
6+
import socket
67
import subprocess
78
import sys
89
from types import SimpleNamespace
@@ -13,6 +14,7 @@
1314
from loopx.file_lock import (
1415
LOCK_ACQUIRE_TIMEOUT_ERROR_CODE,
1516
LockAcquireTimeoutError,
17+
_safe_label,
1618
exclusive_cross_runtime_file_lock,
1719
exclusive_file_lock,
1820
fcntl,
@@ -91,6 +93,9 @@ def test_exclusive_lock_persists_public_safe_holder_metadata(tmp_path: Path) ->
9193
holder_path = lock_holder_path(target)
9294
holder = json.loads(holder_path.read_text(encoding="utf-8"))
9395
assert holder["pid"] > 0
96+
# The record names its own machine, so a reader on another host that
97+
# shares this runtime root never treats the pid as local.
98+
assert holder["host"] == _safe_label(socket.gethostname(), fallback="unknown")
9499
assert holder["agent_id"] == "agent-a"
95100
assert holder["operation"] == "todo-update"
96101
assert holder["acquired_at"].endswith("Z")
@@ -122,9 +127,18 @@ def test_stalled_holder_times_out_and_records_independent_incident(tmp_path: Pat
122127
assert payload["incident_recorded"] is True
123128
incident = payload["lock_timeout"]
124129
assert incident["holder"]["pid"] == process.pid
130+
assert incident["holder"]["host"] == _safe_label(
131+
socket.gethostname(), fallback="unknown"
132+
)
125133
assert incident["holder"]["agent_id"] == "holder-agent"
126134
assert incident["waiter"]["agent_id"] == "waiter-agent"
127135
assert incident["waiter"]["waited_seconds"] >= 0.1
136+
# The refused waiter learns which machine holds the pid, so the operator
137+
# looks for it on the right host instead of assuming it is local.
138+
assert (
139+
incident["operator_action"]["holder_host"]
140+
== incident["holder"]["host"]
141+
)
128142
assert incident["operator_action"]["retry_mode"] == (
129143
"manual_after_holder_inspection"
130144
)

‎tests/test_turn_lane_fence.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88

99
from __future__ import annotations
1010

11+
import socket
1112
from pathlib import Path
1213

1314
from loopx.control_plane.turn_driver.executor import run_loopx_turn_once
15+
from loopx.file_lock import _safe_label
1416
from loopx.control_plane.turn_driver.lane_fence import (
1517
REMEDY_WAIT_FOR_IN_FLIGHT_TURN,
1618
TURN_LANE_IN_FLIGHT,
@@ -119,7 +121,10 @@ def test_the_holder_readback_stays_public_safe(tmp_path: Path) -> None:
119121
assert holder["agent_id"] == AGENT_ID
120122
assert holder["operation"] == TURN_LANE_OPERATION
121123
assert isinstance(holder["pid"], int)
122-
assert set(holder) == {"agent_id", "operation", "pid", "acquired_at"}
124+
# The machine name travels with the pid: two hosts can share one runtime
125+
# root, and a pid without its host is not an actionable identity.
126+
assert holder["host"] == _safe_label(socket.gethostname(), fallback="unknown")
127+
assert set(holder) == {"agent_id", "operation", "pid", "acquired_at", "host"}
123128
# The private lock identity and the runtime path never leave the process.
124129
assert str(tmp_path) not in str(holder)
125130
assert turn_lane_holder_readback(tmp_path / "absent.lane") == {}

0 commit comments

Comments
 (0)