fix(worker): never reuse a backend process whose directory a reinstall replaced#11029
Merged
Conversation
…l replaced
A backend reinstall could poison every subsequent model load on a worker
node until the worker process was restarted.
gallery.InstallBackend (and gallery.UpgradeBackend) replace a backend by
renaming the live directory to `<name>.install-backup`, moving the staged
directory into place, then deleting the backup. A working directory
follows the inode across a rename, so a backend process that outlives
that swap ends up with a deleted inode as its CWD, and every getcwd(2)
in it fails with ENOENT.
Observed on a Jetson Thor worker in distributed mode after two
successive reinstalls of cuda13-nvidia-l4t-arm64-longcat-video-development.
A later model load failed with:
rpc error: code = Internal desc = failed to load LongCat model: [Errno 2] No such file or directory
The backend's own traceback shows it dying while importing torch, before
touching any model file:
backend.py line 142 in LoadModel
backend.py line 300 in _import_torch
torch/_library/custom_ops.py lib._register_fake(...)
torch/library.py:183 caller_module = inspect.getmodule(frame)
inspect.py:1013 f = getabsfile(module)
inspect.py:983 return os.path.normcase(os.path.abspath(_filename))
<frozen posixpath>, line 415, in abspath
FileNotFoundError: [Errno 2] No such file or directory
os.path.abspath calls os.getcwd() for a relative path. Scanning /proc
inside the worker container found the deleted CWD directly:
pid 23467 CWD DELETED: /backends/cuda13-nvidia-l4t-arm64-longcat-video-development.install-backup (deleted)
Restarting the worker container cleared it (dead CWD count 1 -> 0).
Python backends import torch lazily inside LoadModel, so such a survivor
still answers HealthCheck and keeps its gRPC port. It looks healthy and
only detonates when a model is actually loaded through it.
The install paths already stop running processes before replacing the
directory (installBackend's force branch, upgradeBackend, backend.delete),
but they resolve them by name. That bookkeeping reaps nothing whenever
the recorded name no longer resolves into the install's identity set: a
legacy entry with an empty backendName, backendIdentity degraded to
name-only matching after a ListSystemBackends failure, or an earlier
reinstall having already rewritten the metadata.json that carries the
alias. Any of those leaves a live process whose directory is about to be
unlinked, and nothing downstream notices, because the reuse gate checks
liveness and name -- and the name is precisely what does not change
across a reinstall.
Record the directory each supervised process runs out of, plus that
directory's identity at spawn time, and compare with os.SameFile before
reusing the process. This needs none of the name bookkeeping to have
been correct. Both reuse gates are covered: processMatchesBackend (the
install fast path) and startBackend's own already-running branch, which
now force-stops such a survivor so the fresh spawn chdirs into the newly
installed directory. Processes with no recorded directory are accepted,
so a rollout does not restart every running backend once.
This matters more with #11024 pending: making GPU backends visible to
the upgrade checker will have AutoUpgradeBackends fan upgrades out to
worker nodes at scale, and every one of those is a reinstall. Left as
is, a rare manual-upgrade footgun becomes a fleet-wide one.
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]
Contributor
|
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
2 participants
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.
The bug
A backend reinstall could poison every subsequent model load on a worker node until the worker process was restarted.
gallery.InstallBackend(core/gallery/backends.go:307-467) andgallery.UpgradeBackend(core/gallery/upgrade.go:320-333) replace a backend by renaming the live directory to<name>.install-backup, moving the staged directory into place, then deleting the backup. A working directory follows the inode across a rename, so a backend process that outlives that swap ends up with a deleted inode as its CWD, and everygetcwd(2)in it fails with ENOENT.pkg/model/process.go:250-269starts every backend withWithWorkDir(filepath.Dir(grpcProcess)), i.e. the backend directory itself — which is the directory the reinstall unlinks.Evidence
Observed on a Jetson Thor worker in distributed mode after two successive reinstalls of
cuda13-nvidia-l4t-arm64-longcat-video-development(an upgrade, then a pin to a specific image). A later model load failed with:The backend's own traceback (via
GET /api/nodes/{id}/backend-logs/{modelId}) shows it dying while importing torch, before touching any model file:os.path.abspathcallsos.getcwd()for a relative path. Scanning/procinside the worker container found the deleted CWD directly:Restarting the worker container cleared it (dead CWD count 1 -> 0).
longcat-video/backend.pyimports torch lazily, insideLoadModel(self._import_torch()), so such a survivor still answersHealthCheckand keeps its gRPC port. It looks perfectly healthy and only detonates when a model is actually loaded through it.Why the existing guards did not catch it
The install paths already stop running processes before replacing the directory —
installBackend's force branch andupgradeBackend(core/services/worker/install.go), andbackend.delete(core/services/worker/lifecycle.go:191-231, the recent stale-process/orphan fix). All of them resolve processes by name, viaresolveProcessKeysForBackend+backendIdentity.That bookkeeping reaps nothing whenever the recorded name no longer resolves into the install's identity set:
backendName;backendIdentitydegrading to name-only matching after aListSystemBackendsfailure, orsystemState == nil(supervisor.go:68-79);metadata.jsonthat carries the alias — which fits the observed two successive reinstalls exactly: the first install rewrites the metadata, so the second install's alias resolution can no longer reach a process recorded under the pre-upgrade name.Any of those leaves a live process whose directory is about to be unlinked, and nothing downstream notices: the reuse gate checks liveness and name, and the name is precisely what does not change across a reinstall. So the recent stale-process fix covers the deleted-backend orphan and the different-backend slot reuse, but not directory replacement under the same name.
The fix
Record the directory each supervised process runs out of, plus that directory's identity at spawn time, and compare with
os.SameFilebefore reusing the process. This needs none of the name bookkeeping to have been correct.Both reuse gates are covered:
processMatchesBackend— the install fast path;startBackend's own already-running branch, which now force-stops such a survivor so the fresh spawnchdirs into the newly installed directory.Processes with no recorded directory are accepted, so a rollout does not restart every running backend once. Because the guard sits at the reuse gate rather than in the gallery layer, it covers both swap sites (
InstallBackendandUpgradeBackend) and the delete path equally.Alternatives considered and rejected: spawning backends in a fixed neutral directory would break
run.sh, which resolvesbackend_dir=$(dirname "$0")against a relative$0and therefore depends on the CWD being the backend directory; and adding yet another name-keyed stop-before-swap would repeat the class of bug that let this one through.Why this matters now
#11024 makes GPU backends visible to the upgrade checker for the first time, which will have
AutoUpgradeBackendsfan upgrades out to worker nodes at scale. Every one of those upgrades is a backend reinstall. Left as is, a rare manual-upgrade footgun becomes a fleet-wide one.Tests
TDD, Ginkgo/Gomega.
core/services/worker/backend_reinstall_cwd_test.gosimulatesInstallBackend's rename-install-delete swap verbatim and asserts the process may no longer be reused. Red before the fix on behaviour:Two control specs (untouched directory; legacy entry with no recorded directory) were green throughout.
go build ./core/... ./pkg/...— exit 0go test ./core/services/worker/... ./core/gallery/... ./pkg/model/...— exit 0make lint— exit 0, 0 issuesKnown gap, deliberately out of scope
The single-node (non-distributed) path has the same hazard and no guard at all:
LocalBackendManager.UpgradeBackend(core/services/galleryop/managers_local.go:112) callsgallery.UpgradeBackendwithout stopping any model process using that backend, and the local reuse gateModelLoader.checkIsLoaded(pkg/model/loader.go:624) checks health/liveness only. Fixing that needs a per-Modelbackend-directory record inpkg/modelplus its own gate and tests; worth a follow-up.Making the error itself less baffling (detecting "the CWD vanished" and reporting it as such instead of a bare
[Errno 2]from inside torch) belongs in the Python backend layer, not here — this change prevents the poisoned reuse that produced it in the first place.