Skip to content

Migrate execution coordination from HTTP to WebSockets - #470

Merged
hermabr merged 22 commits into
mainfrom
t3code/migrate-http-to-websockets
Aug 2, 2026
Merged

Migrate execution coordination from HTTP to WebSockets#470
hermabr merged 22 commits into
mainfrom
t3code/migrate-http-to-websockets

Conversation

@hermabr

@hermabr hermabr commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace the execution coordinator HTTP API with a persistent WebSocket protocol.
  • Add authenticated worker handshakes, protocol versioning, job assignment, results, and shutdown messages.
  • Update local and Slurm backends to use the coordinator directly for pool operations.
  • Remove FastAPI, HTTPX, and Uvicorn dependencies in favor of websockets.

- Replace the FastAPI/HTTP API with a versioned WebSocket worker protocol
- Remove HTTP client dependencies and coordinate local worker pools directly
hermabr and others added 13 commits July 31, 2026 21:36
- Use ExecutionCoordinator directly for worker backend coordination
- Update protocol discriminators and tests
- Simplify hello messages and server connection handling
- Remove obsolete protocol version validation test
lease_job now blocks on the coordinator's condition until a job is
available or the run is done, deleting the "wait" token and the server's
1s wake poll (and with it a missed-wakeup race). The lock and wake
primitives merge into a single Condition notified by mutators.

The strictly sequential worker channel drops its envelopes: workers send
a bare JobResult instead of ResultMessage, hellos validate directly, and
worker_message_adapter goes away. lease_job also stops releasing prior
leases per request - connection teardown (worker_lost) is the single
release path. client.py folds into its only caller, worker_loop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

hermabr commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

hermabr and others added 2 commits August 1, 2026 16:19
Idle worker handlers block in lease_job without touching the socket, so
closing their connections never woke them: an interrupted run waited out
the 10s drain timeout and leaked non-daemon handler threads that hang
interpreter exit. Server shutdown now marks the coordinator finished
first, which wakes every waiting handler, and the run loop closes the
server before joining worker pools so pool stops only reap
already-exiting workers.

With that, the explicit stop/assign envelopes are unnecessary: the
server sends Job frames directly and a normal WebSocket close is the
stop signal, removing AssignMessage, StopMessage, their kind
discriminators, and the server message adapter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hermabr and others added 3 commits August 1, 2026 17:15
Only one job can be outstanding per connection and the server retains
the Job it sent until the matching result arrives, so per-member UUID
lease IDs carry no information. Key the coordinator's running table by
object id (unique among running jobs because nodes are popped from
ready) and send Job.artifacts as a plain list of ArtifactSpec.

This deletes JobMember, RunningJob.lease_id, FailedJob.lease_id, UUID
generation, and the lease-centric log details.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hermabr and others added 2 commits August 1, 2026 17:29
The dynamic scaler (scale thread, polling interval, failed-thread
bookkeeping, restart budget, idle timeouts forcing workers to exit and
be recreated) was built for the HTTP era when idle workers polled. An
idle WebSocket worker is just a blocked thread, so start max_workers
threads once, keep them connected with no idle timeout, and let the
server closing the connection end them.

A crashed worker thread now fails the run immediately: job failures are
already caught inside the worker loop, so a thread-level crash means a
furu bug, not a job problem.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three failure budgets overlapped: coordinator per-object retries,
worker consecutive-job failures, and pool failed-worker restarts. The
worker threshold (5) rarely fired under the coordinator's retry
default (3), and when it did the worker exited zero, Slurm recorded a
normal completion, and the pool replaced it without consuming any
restart budget. The Slurm restart budget itself could never govern
anything because the scale loop fails the whole run on the first
failed Slurm state.

Keep two policies: the coordinator owns job failure and retry; the
pool owns worker/allocation failure. Remove max_consecutive_failures
(worker loop, CLI flag, sbatch script plumbing), the unused Slurm
restart budget, the now-unused worker.max_failed_restarts config, and
the never-read SlurmWorkerPool._server_url/_auth_token fields.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

hermabr commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Merge activity

  • Aug 2, 12:45 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 2, 12:45 AM UTC: @hermabr merged this pull request with Graphite.

@hermabr
hermabr merged commit c72c21c into main Aug 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant