mirror of
https://github.com/NVIDIA/Model-Optimizer.git
synced 2026-10-02 03:14:52 +08:00
[tools/debugger] Enforce a single relay owner across hosts (#1735)
### What does this PR do? Type of change: Bug fix The `tools/debugger` file-based relay assumes a single server but never enforced it. Because the relay lives on shared NFS (the repo is often the same checkout mounted on multiple hosts), a forgotten `server.sh` on another host kept polling the same `.relay/` and could **steal commands** (executing them on the wrong host), and killing one server's cleanup could **wipe the active server's markers**. This adds a `.relay/owner` ownership token (`host:pid:nanos`): - Each server writes `owner` atomically at startup and **takes over** instead of refusing when a stale `server.ready` exists (the old `kill -0 <pid>` guard was host-local and meaningless across hosts). - The handshake and main loops exit cleanly if `owner` changes (`[server] Superseded by <id> — exiting.`), so a freshly started server **evicts** any stale one — even on another host. - `cleanup()` only clears shared markers if we still own them, so a stepping-down server never clobbers its successor's `server.ready`/`owner`. Also gitignores `tools/debugger/logs/` and documents the `owner` file in the README. ### Usage ```bash # Inside the container; a previously-running server elsewhere that shares this # NFS .relay/ steps down automatically once this one claims ownership: bash tools/debugger/server.sh # [server] Note: existing server.ready found (<host:pid:ts>); taking over. # (the stale server logs: "[server] Superseded by <id> — exiting.") ``` ### Testing Verified live on computelab: a forgotten `server.sh` on another host was evicted when a new server started, after which `client.sh run` executed on the correct (new) host; confirmed the stepping-down server's cleanup does not remove the successor's `server.ready`/`owner`. `server.sh` passes `bash -n`. ### Before your PR is "*Ready for review*" - Is this change backward compatible?: ✅ <!-- additive: new .relay/owner file; client.sh and the wire protocol are unchanged --> - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: N/A - Did you write any new necessary tests?: N/A <!-- the file-based relay tool has no test harness; behavior verified manually --> - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A <!-- internal dev tooling, not a shipped feature/API --> - Did you get Claude approval on this PR?: N/A <!-- can run /claude review --> ### Additional Information Scope is limited to `tools/debugger/` (`server.sh`, `README.md`, `.gitignore`). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated relay protocol documentation to clarify ownership-based server coordination. * **Bug Fixes** * Improved reliability of multi-server coordination in shared relay environments. * **Chores** * Updated ignore patterns for logging files. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6968fe78fd
commit
d7df14d12a
@@ -1 +1,2 @@
|
||||
.relay/
|
||||
logs/
|
||||
|
||||
@@ -66,6 +66,7 @@ The relay uses a directory at `tools/debugger/.relay/` with this structure:
|
||||
```text
|
||||
.relay/
|
||||
├── server.ready # Written by server on startup
|
||||
├── owner # Current relay owner id (host:pid:nanos); newest server wins
|
||||
├── client.ready # Written by client during handshake
|
||||
├── handshake.done # Written by server to confirm handshake
|
||||
├── running # Written by server while a command is executing (cmd_id:pid)
|
||||
@@ -118,7 +119,9 @@ The relay uses a directory at `tools/debugger/.relay/` with this structure:
|
||||
## Notes
|
||||
|
||||
- The `.relay/` directory is in `.gitignore` — it is not checked in.
|
||||
- Only one server should run at a time (startup clears the relay directory).
|
||||
- Only one server serves at a time. A newly started server claims `.relay/owner`;
|
||||
any older server (even on another host sharing this NFS relay) sees the changed
|
||||
owner on its next poll and exits cleanly, rather than competing for commands.
|
||||
- Commands run sequentially in the order the server discovers them.
|
||||
- A running command can be cancelled via `client.sh cancel`. Cancelled commands exit with code 130.
|
||||
- Client-side timeouts automatically cancel the running command on the server.
|
||||
|
||||
+31
-10
@@ -63,6 +63,12 @@ RESULT_DIR="$RELAY_DIR/result"
|
||||
|
||||
echo "[server] Workdir: $WORKDIR"
|
||||
|
||||
# Unique id for this server instance. Servers can share one relay dir over NFS
|
||||
# (e.g. the same repo mounted on multiple hosts), so single-owner is enforced by
|
||||
# this token rather than by host-local PIDs: whoever writes .relay/owner last wins,
|
||||
# and the others step down on their next poll (see below).
|
||||
SERVER_ID="$(hostname):$$:$(date +%s%N)"
|
||||
|
||||
cleanup() {
|
||||
echo "[server] Shutting down..."
|
||||
# Kill any running command (guard all reads with || true to prevent set -e
|
||||
@@ -73,10 +79,12 @@ cleanup() {
|
||||
fi
|
||||
# Kill any child processes in our process group
|
||||
pkill -P $$ 2>/dev/null || true
|
||||
rm -f "$RELAY_DIR/server.ready"
|
||||
rm -f "$RELAY_DIR/handshake.done"
|
||||
rm -f "$RELAY_DIR/running"
|
||||
rm -f "$RELAY_DIR/cancel"
|
||||
# Only clear shared markers if we still own the relay — never clobber a
|
||||
# successor server that has taken over (servers share one NFS relay dir).
|
||||
if [[ "$(cat "$RELAY_DIR/owner" 2>/dev/null)" == "$SERVER_ID" ]]; then
|
||||
rm -f "$RELAY_DIR/server.ready" "$RELAY_DIR/handshake.done" \
|
||||
"$RELAY_DIR/running" "$RELAY_DIR/cancel" "$RELAY_DIR/owner"
|
||||
fi
|
||||
exit 0
|
||||
}
|
||||
trap cleanup SIGINT SIGTERM
|
||||
@@ -84,19 +92,20 @@ trap cleanup SIGINT SIGTERM
|
||||
# Set environment
|
||||
export PYTHONPATH="$WORKDIR"
|
||||
|
||||
# Check for an already-running server
|
||||
# A previously-running server (possibly on another host sharing this NFS relay)
|
||||
# steps down on its next poll once we claim ownership below, so take over rather
|
||||
# than refuse to start.
|
||||
if [[ -f "$RELAY_DIR/server.ready" ]]; then
|
||||
old_pid=$(cut -d: -f2 "$RELAY_DIR/server.ready")
|
||||
if kill -0 "$old_pid" 2>/dev/null; then
|
||||
echo "[server] ERROR: Another server (PID $old_pid) is already running."
|
||||
exit 1
|
||||
fi
|
||||
echo "[server] Note: existing server.ready found ($(cat "$RELAY_DIR/server.ready" 2>/dev/null)); taking over."
|
||||
fi
|
||||
|
||||
# Initialize relay directories
|
||||
rm -rf "$RELAY_DIR"
|
||||
mkdir -p "$CMD_DIR" "$RESULT_DIR"
|
||||
|
||||
# Claim ownership of the relay (single-owner enforcement; see main loop).
|
||||
echo "$SERVER_ID" > "$RELAY_DIR/owner.tmp" && mv "$RELAY_DIR/owner.tmp" "$RELAY_DIR/owner"
|
||||
|
||||
# Ensure modelopt is editable-installed from WORKDIR
|
||||
check_modelopt_local() {
|
||||
python3 -c "
|
||||
@@ -129,6 +138,11 @@ echo "[server] Waiting for client handshake..."
|
||||
|
||||
# Wait for client handshake
|
||||
while [[ ! -f "$RELAY_DIR/client.ready" ]]; do
|
||||
current_owner="$(cat "$RELAY_DIR/owner" 2>/dev/null || true)"
|
||||
if [[ -n "$current_owner" && "$current_owner" != "$SERVER_ID" ]]; then
|
||||
echo "[server] Superseded by $current_owner before handshake — exiting."
|
||||
exit 0
|
||||
fi
|
||||
sleep "$POLL_INTERVAL"
|
||||
done
|
||||
|
||||
@@ -140,6 +154,13 @@ echo "[server] Handshake complete. Listening for commands..."
|
||||
# Main loop: watch for command files and re-handshake requests
|
||||
shopt -s nullglob
|
||||
while true; do
|
||||
# Step down if a newer server has claimed the relay (single-owner across hosts).
|
||||
current_owner="$(cat "$RELAY_DIR/owner" 2>/dev/null || true)"
|
||||
if [[ -n "$current_owner" && "$current_owner" != "$SERVER_ID" ]]; then
|
||||
echo "[server] Superseded by $current_owner — exiting."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Detect re-handshake (client flushed and reconnected)
|
||||
if [[ -f "$RELAY_DIR/client.ready" && ! -f "$RELAY_DIR/handshake.done" ]]; then
|
||||
CLIENT_INFO=$(cat "$RELAY_DIR/client.ready")
|
||||
|
||||
Reference in New Issue
Block a user