Repository navigation
fix(hub): keep the local sidecar running through a hub restart - #184
Closed
TheGreatAxios wants to merge 2 commits into
Closed
TheGreatAxios wants to merge 2 commits into
TheGreatAxios wants to merge 2 commits into
Conversation
A hub restart (SIGTERM, a fatal error, a failed start or a crash) now leaves the local sidecar running, and the next hub adopts it when the recorded pid is still the same process (start time and command), so it reconnects over Interchange's stock path and a turn in flight carries on. SIGINT, as when the dev script exits, still stops it. The e2e harness can restart the hub in place and hold a recorded model reply.
Tag the local sidecar and a new watchdog with the data dir and a fingerprint of their spawn inputs; adopt only an exact match, stop every other process on the data dir, replace an adopted sidecar that does not reconnect within a minute, and stop an orphan after ten minutes without a hub. Keep the graceful stop when the hub is process 1 or SIDECAR_LOCAL_OUTLIVES_HUB=false, release the child so a SIGTERM'd hub exits, and restart a real hub process in the e2e test.
Contributor
Author
|
Closing: the approach adds custom sidecar lifecycle code in Builder that belongs in a provisioner plugin and Interchange. The scope of CL-12139 will be re-defined before any new PR. |
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 hub restart no longer stops the local sidecar. When the hub gets SIGTERM, hits a fatal error, fails to start or crashes, the sidecar keeps running. The next hub adopts it, and it reconnects through Interchange's stock path (hello, route restore, welcome, queued mail), so a turn in flight carries on. Interchange itself is unchanged.
The hub now starts the sidecar under a small watchdog (
apps/hub/src/sidecar/watchdog.ts). Both processes carry the hub's data dir on their command line, and the watchdog also carries a fingerprint of everything the sidecar was spawned from: hub URL, identity, token, data dir, credential key, deployment limit, child environment, adapter manifest and the vendored Interchange revision. At boot, the hub lists processes. It adopts the one sidecar on its data dir whose fingerprint matches and stops every other process on that data dir, so two sidecars never share one. A sidecar left by a hub from before this change, which only recorded a pid, is stopped as before. Before signalling an adopted process, the hub checks that the pid still runs the same command.A sidecar is now stopped in these cases:
SIDECAR_LOCAL_OUTLIVES_HUB=false, or is process 1 and leaves it unset, which keeps the graceful SIGTERM-then-SIGKILL stop for containers (the Dockerfile sets it to false);The hub's SIGTERM shutdown releases the sidecar's process handle, so the hub exits rather than waiting on its child.
The e2e harness can now run the hub as its own
server.tsprocess and restart it with SIGTERM. The recorded model server can also hold a reply. The new hub-restart e2e test holds a writer's model reply, sends the hub process SIGTERM, starts a new one, then releases the reply. It checks that the writer files its document, no run is listed as failed, and the coordinator gets no extra exchange. WithSIDECAR_LOCAL_OUTLIVES_HUB=false, the same test fails because the coordinator receives "The writer stopped before finishing." The test delays the coordinator's first approval request by a second, because the hub compares a Postgres timestamp with its own clock; a separate fix removes that.I ran lint, format:check, typecheck,
bun run test, pins:verify, vendor:verify and the full e2e suite against a throwaway Postgres, and all passed. I also checked by hand that the watchdog stops a sidecar when no hub answers. With the reconnect check shortened to three seconds, the e2e test still passed, so an adopted sidecar that reconnects is not replaced.Refs CL-12139