fix(worker): survive a transient failure at startup instead of exiting - #174
Draft
nilsmechtel wants to merge 1 commit into
Draft
fix(worker): survive a transient failure at startup instead of exiting#174nilsmechtel wants to merge 1 commit into
nilsmechtel wants to merge 1 commit into
Conversation
Two startup paths turned a momentary dependency failure into a worker self-termination. The initial connect had no retry. hypha_rpc and the Ray client both reconnect an established connection, but neither retries the first one, so a worker started inside a server restart window exited on the first refusal and Kubernetes restarted it straight back into the same window. connect_with_retry now wraps both connect calls with a bounded backoff (120 s budget, 2 s doubling to 15 s) and only retries connection-level failures, so token, workspace and client-id errors still fail fast. Startup applications were deployed with a bare call in a loop, so one application's failure propagated into start()'s blanket handler and took the whole worker down with it. Each application is now isolated: a failure is logged, the remaining applications still deploy, and the failed ones are retried in the background on the same schedule monitor_applications uses. The monitor cannot pick them up itself — a failure inside deploy_app leaves no entry in _deployed_applications. Closes the two failure modes behind the observed deNBI ENOLCK and KTH ECONNREFUSED self-terminations.
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
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.
A worker that starts during a dependency's bad minute now waits it out instead of exiting. Two independent paths caused self-terminations at both KTH and deNBI, and both are fixed here — they are different files but the same claim, so they are easier to judge together than apart.
Closes the two failure modes tracked as
#0014(initial connect, no retry) and#0015(one startup application's failure is fatal to the worker) in the session backlog.The initial connect had no retry
hypha_rpcand the Ray client both reconnect an established connection, but neither retries the first one. The KTH evidence is unambiguous — zero elapsed seconds betweenConnecting to Hypha server at …andERROR Failed to start BioEngine worker: [Errno 111] Connect call failed, thenexitCode 1. One attempt, immediate refusal, immediate exit. Kubernetes restarted the pod straight back into the same ~20-45 s hypha-server restart gap.connect_with_retry(bioengine/utils/network.py) now wraps both connect calls:connect_to_serverinBioEngineWorker._connect_to_serverandray.initinRayCluster._connect_to_cluster. The default budget is 120 s, starting at 2 s and doubling to a 15 s cap — comfortably past the observed restart window, and well under the startup probe's own 9-minute budget, so a waiting worker is not killed by its own probe.It retries connection-level failures only.
ConnectionError,TimeoutErrorandsocket.gaierrorby type, plus a small set of message markers for the failures that are not raised as anOSErrorsubclass (HTTP 503from a server that is up but not yet serving, the Ray client's own connect timeout). Everything else is treated as fatal and re-raised on the first attempt, which keeps the checks that must fail fast failing fast: the admin-permissionValueErroratworker.py:576, the workspace mismatch at:583, the client-id mismatch at:589, and an expired worker token. A test asserts exactly this asymmetry rather than asserting the retry alone.Only
ray.initis retried on the Ray side, not the rest of_connect_to_cluster. That is deliberate:ray.initis the first thing the method does, so a retry is genuinely a fresh attempt. Retrying after the proxy actor exists would hitRay is already initialized— which is not classified as transient, so it propagates rather than looping.One consequence worth reviewing:
_connect_to_serveris also the monitoring loop's reconnect path (_check_hypha_connection), so a reconnect can now occupy a monitoring tick for up to 120 s instead of failing fast. That is the intended trade — wait out a Hypha restart rather than churn — and the degraded-threshold backstop still trips if the reconnect ultimately keeps failing, because the tick error counter is unchanged.One startup application's failure was fatal to all of them
deploy_startup_applications()calleddeploy_appbare in a loop. The exception propagated toworker.py:1053, insidestart()'s try block, and was caught by the blanket handler that shuts the worker down. On deNBI on 2026-08-28 a single[Errno 37] No locks available— NFS lockd on the shared PVC, during model-runner's build, nothing to do with the other two applications — took the whole worker with it. An earlierNoSuchKeyon artifact resolve did the same on 08-24, and one mispinned application produced a 64-restart burst.Each application is now isolated: the failure is logged, the remaining applications still deploy, and the worker comes up.
The recovery half needs a note, because the issue's suggested fix does not work as written. It proposed handing the failure to
monitor_applications, which already runs a per-app 10/20/40 … 600 s backoff. It cannot:monitor_applicationswalks_deployed_applications, and a failure insidedeploy_appraises before the entry is created (manager.py:2533), so there is nothing for the monitor to see. Note the contrast with a failure inside the background_deploy_applicationtask — that one does leave the entry in place and is already covered by the monitor today.So
_retry_startup_applicationsretries the pre-registration failures in the background, reusing_REDEPLOY_BACKOFF_INITIAL_SECONDS/_REDEPLOY_BACKOFF_MAX_SECONDSrather than introducing a second schedule. Without it the fix would trade one bad property for another: the worker would survive, but the failed application would stay down until a human redeployed it, where today the crash-loop is at least a crude retry. The task is cancelled in_cleanupso a pending retry cannot fire into a teardown — in both cluster modes, since_cleanupskipsstop_all_appsentirely in external-cluster mode.Config validation is deliberately left fatal. A bad key or a missing
artifact_idin the startup config is an operator mistake that will not fix itself, and failing fast surfaces it immediately.Verification
tests/worker/test_startup_resilience.py, 7 tests, all passing. Both halves have a positive control — the fix was neutered one edit at a time and exactly one test flipped each time, with the exact error from the corresponding production log:except Exceptionto a type that does not match failed onlytest_one_failing_startup_application_does_not_abort_the_others, withOSError: [Errno 37] No locks availabletest_worker_retries_the_initial_hypha_connection, withConnectionRefusedError: [Errno 111] Connect call failedThat second test drives
BioEngineWorker._connect_to_serverrather than the helper in isolation, so it proves the retry is wired in, not merely available. The Ray call site has no equivalent test — mockingray.initplus the proxy actor was more scaffolding than the assertion is worth — so that one is verified by inspection only.97 nearby unit tests still pass (
tests/worker,tests/apps,tests/test_gpu_sizing.py).Not yet validated on a live cluster. This touches the startup path in every mode, so it wants a dev-image run on a real worker before it goes ready.