Rolling updates for versiond-managed binaries (blue/green + drain, part 1 of #1367)#1437
Conversation
0213646 to
ea882d3
Compare
071f9c1 to
03129e4
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03129e4b64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
30s (DrainKillGrace): “we don’t know inflight; don’t stall 15m; give a short cushion then stop asking.” Docs table line that says
|
|
But as I understand versioned graceful stop or devshardd host stop is deffered for next PR |
|
Also at Consider adding a testenv scenario (e.g. S7), for testing that in-flight session keep living while version swap, node replacement, downing node. It could be used for following cases:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62271b9859
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 714ba4672f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e056edcb7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75690a188a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
75690a1 to
1dc1f08
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dc1f086d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Install each approved binary by sha256, start replacement children on a new port, wait for readiness, atomically move routing to the new child, and drain the old child before stopping it. Add devshardd lifecycle endpoints, proxy health metadata, and e2e coverage for same-name sha256 swaps.
Recheck child restart state after backoff so a draining old child cannot restart after route promotion, and shorten legacy drain waits when /drain/status is unavailable. Fail closed on transient preflight probe errors while preserving legacy unsupported-flag fallback, and validate oracle version names before reconciling.
Use testify assertions in the new devshard event listener test, prefer any in the versioned e2e test app JSON maps, and rename childStart.context to ctx.
Validate oracle-provided sha256 values before reconciliation so invalid hashes cannot flow into install paths or cleanup. This keeps bad oracle responses fail-closed while leaving currently running children unchanged. Add focused process tests for failed replacement readiness and drain idle/timeout behavior, and update the e2e hash mismatch case to use a syntactically valid wrong checksum. Document the legacy preflight, readiness, and drain fallbacks used during the first versiond upgrade.
Build version-stamped testapp artifacts for v0.2.11, v0.2.12, and v0.2.11-rollout so versiond preflight can validate protocol versions in the full-stack fixture. Extend VersiondTests with a same-version binary update that keeps a slow request on the old child, waits for route swap to rollout, checks continuity probes, and waits for the draining child to exit.
Do not reuse the 10s HTTP shutdown context for child process teardown. Give the process manager its own shutdown budget based on the per-child stop timeout so DEVSHARD_SHUTDOWN_GRACE remains effective for devshardd children. Log incomplete HTTP or manager shutdown paths instead of silently ignoring those errors.
Keep /ready, /drain, and /drain/status off the public `devshardd` listener and start a private admin listener when versiond provides DEVSHARD_ADMIN_ADDR. Probe --print-admin-api-version for devshardd children, allocate a loopback admin port only for binaries that advertise support, and keep legacy children on the existing public lifecycle fallback. Use 127.0.0.1 consistently for child lifecycle probes and strip inherited DEVSHARD_ADMIN_ADDR before setting per-child env.
Raise VERSIOND_DRAIN_KILL_GRACE to 10m so the first legacy-to-new devshardd swap gives old in-flight work a real drain window before SIGTERM. Keep the default in one exported config constant so Load and manager normalization stay aligned. Clarify the overloaded knob semantics in the rolling update design: legacy no-status cushion, non-devshard child stop timeout, and the minimum devshardd stop timeout combined with DEVSHARD_SHUTDOWN_GRACE.
Move children for removed approved versions into the draining set instead of cancelling them synchronously from Reconcile. This removes the route immediately, disables restart for the old child, and lets drainAndStop handle the in-flight wait and legacy drain cushion in the background. Cover the non-blocking removal path and the legacy no-status grace path with manager unit tests, and document the removal semantics in the rolling update design.
Replace the monotonic child port counter with a bounded pool that scans from BasePort and reuses ports after child exit. Reserve versiond's own listen port so long-lived supervisors cannot eventually assign :8080 to a child, and normalize invalid BasePort values back to the default. Update manager tests for port reuse, reserved-port skipping, and out-of-range BasePort handling. Document the bounded child-port pool in the rolling update design.
Add a devshardd --print-storage-mode capability so versiond can ask the actual child binary how it resolves DEVSHARD_STORAGE_MODE. Record the running child's storage mode during preflight and allow blue/green overlap only when both the running and incoming devshardd binaries report postgres. Fall back to stop/start for legacy, hybrid, sqlite, unknown, or failed probes. Remove the old versiond-side SQLite probe and its modernc.org/sqlite dependency. Update rolling-update docs and tests for the new fail-closed storage-mode gate.
1dc1f08 to
9d84158
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d841580fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add an S9 citest scenario that updates an approved binary under the same version name and verifies request continuity, blue/green overlap, new-request routing, and eventual drain on both versiond hosts. Cover the hybrid storage fallback separately to prove that unsafe storage modes use stop/start without overlapping children. Let Docker assign isolated host ports for concurrent citest stacks and extend mock-dapi with mutable approved versions and binary serving. Update the testenv documentation for the new scenario and harness.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f7f340f08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Register a request in the lifecycle inflight counter before checking the draining flag. Requests that race with drain are now either visible to versiond or rejected with their temporary count removed. Cover the rejection path by asserting drain status returns to zero.
Replace string routes with generation-specific proxy targets that lease each forwarded request. Retired targets reject stale acquisitions while already acquired requests remain pinned to the old child. Wait for old proxy leases before requesting child drain, using the same VERSIOND_DRAIN_TIMEOUT for proxy and lifecycle phases. Apply the gate to rolling swaps and removed versions. Cover target retirement, stale-route retry, route-swap continuity, and manager drain ordering. Update the rolling update design.
Keep restored approved versions from starting while an older child with the same name is still draining. Recheck the guard before cached starts, downloads, download completion, and local override activation. This prevents multiple generations from sharing a version data directory after a remove-and-readd sequence. Start the restored version on a later reconcile after the draining child exits. Cover normal, in-progress download, and override paths. Document the re-add behavior in the rolling update design.
Require the running child path and archive identity to match the local override before treating equal flat-file contents as already active. This prevents a stale flat override file from masking a child that is actually running from a downloaded per-SHA install. Cover both the stale-file replacement and exact-identity no-op paths.
|
The idea is right, some hardening might be needed, but it's non-blocking |
Resolve verified binaries from both per-SHA and legacy flat layouts. Atomically copy matching legacy installs into the canonical per-SHA directory before starting a child, without requiring network access. Publish install metadata only after verifying the promoted binary and retain the flat source for older supervisors sharing the bin mount. Fall back to the re-verified flat binary when promotion cannot write. Cover successful, concurrent, failed, and invalid promotion paths. Document the upgrade and shared-mount behavior.
Replace CommandContext, WaitDelay, and competing child wait timers with a single process owner that moves through running, terminating, killing, and exited states. Send SIGTERM to the child process group and escalate to SIGKILL after the configured grace period. Close completion only after cmd.Wait has reaped the process. Make manager shutdown stop all children first. Use its context only to force escalation without abandoning process ownership or reap. Cover graceful exit, forced escalation, and shutdown-deadline behavior. Document the process-level contract and its boundary with the future host evacuation FSM.
Gate the child Starting-to-Running transition on logical admin readiness and public HTTP health. Recheck both conditions within one VERSIOND_READY_TIMEOUT. This prevents early admin readiness from hiding a delayed or failed public bind. Keep legacy readiness fallback only for children without the admin API. Require modern children to return 200 from the configured admin path and 2xx from public /healthz before publishing their route. Cover unavailable public listeners and readiness changes between probe attempts. Document the combined promotion gate.
Replace the -1 exhaustion sentinel with an explicit error and propagate it through cached, downloaded, override, and replacement child starts. Stop admin-capable children before exec when their second port cannot be allocated, release the public port, and keep the current rolling target active when replacement allocation fails. Cover pool exhaustion, publication guards, and admin-port cleanup. Document the failure behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e586a2df57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return fmt.Errorf("create replacement version %s: %w", v.Name, startErr) | ||
| } | ||
| m.mu.Unlock() | ||
| go m.runChild(newChild.ctx, newChild.child) |
There was a problem hiding this comment.
When a rolling swap starts the replacement child here, it is not added to m.processes or m.draining until after readiness succeeds at the later swap point. If versiond is shutting down while waitForChildReady is pending, Manager.Shutdown only snapshots those maps, so it can stop/wait for the old child and return without waiting for or force-killing this already-started replacement; in the main shutdown path the signal context may have sent SIGTERM, but versiond can still exit before this goroutine reaps or escalates a child that is slow or ignores SIGTERM. Keep pending replacement children in a tracked set that Shutdown also stops and waits on before starting the process.
Useful? React with 👍 / 👎.
Merge four new commits from origin/ak/pixelplex-refactoring-into-r2. They add the S9 validation lease race, sticky-session failover, gRPC settlement confirmation, and edge-api release wiring. Keep upstream S9 as the numbered validation scenario. Rename the local rolling-update test and its helpers to VersiondRollingUpdate, add the citest-versiond-rolling-update target, and update testenv docs. Resolve endpoint discovery around Docker-assigned host ports so the new S9 topology and rolling-update stacks both use published addresses.
Replace S1-S9 identifiers in citest file names, test functions, harness helpers, temporary resources, and Make targets with behavior-oriented names. Keep citest-stack selecting the same scenarios, add the descriptive citest-validation-lease-race target, and update testenv documentation and plans to use the new names.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb2177765d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| old.Stop() | ||
| waitForChild(old) |
There was a problem hiding this comment.
When rolling overlap is disabled for SQLite/hybrid or legacy-storage devshard children, this branch sends SIGTERM while the old child remains in m.processes and the route table until waitForChild returns. Requests that already acquired, or newly acquire during the shutdown grace, the old proxy target can be forwarded after the child has entered drain or closed its listener, so accepted work can fail instead of draining; unlike the overlap/removal paths, this fallback never retires the target before stopping the child. Move the old child out of routes and wait for proxy leases before calling Stop().
Useful? React with 👍 / 👎.
3 participants
Summary
This PR adds rolling updates for versiond-managed binaries: when governance publishes a
new binary under the same version name (name stays, e.g.
v0.2.13, only thesha256changes), versiond now performs a blue/green swap with drain instead of the current
stop-then-start. In-flight work on the old
devsharddchild is allowed to finish; the newchild receives traffic only after it is actually ready.
It implements Part 1 (Track A) of the
rolling-update plan
and is the first stage of the HA proposal
#1367
("rolling updates first" per maintainer feedback in the discussion).
Motivation
Today a governance sha change is handled by
downloadAndSwapas stop-then-start on thesame port: the old child gets
SIGTERMand isSIGKILLed after 5s, before the new binaryhas proven it can serve. Any inference longer than ~5s (long SSE
/chat/completions,validation) is cut mid-flight.
For operators this is not cosmetic: a dropped in-flight inference means failed/unanswered
validations (Executor Reputation), and mid-epoch unavailability feeds into cPoC
confirmation ratio. The operator requirement (rolling-update.md §0) is:
How a swap works now
If the new child never becomes ready, the swap aborts and the old child keeps serving
(retry on the next poll) — a failed rollout no longer takes the version down.
What ships in this PR
versiond (
versioned/)downloadAndSwaprewritten; per-namecurrent+draining[]children; per-generation port allocation with release; draining children excluded from routes and from the crash-restart loopwaitForReadyprobesVERSIOND_READY_PATH(default/ready); replaces the TCP-only probe and its "routing anyway" fallback; swap aborts (old keeps serving) on readiness failurebin/<name>/<sha256>/(write-once; safe for a draining child's crash-restart); superseded install dirs are GC'd, keeping desired/live/draining plus a small retain cushion; in-progress downloads are never GC'dPGHOSTset and no leftover SQLite-owned sessions in the data dir (probed via_meta.db); otherwise versiond falls back to today's stop-then-start — SQLite single-instance setups keep their current behavior--print-*flags) falls back to the governance slot name on a recognized "unsupported flag" error (fail-closed on timeouts/signals);/readyfalls back to/healthz→ TCP for the default path only; missing/drain/status(404/405/501) shortens the drain wait instead of stalling 15mDevshardEscrowParams.Validate); an invalid response fails the poll and leaves running children untouched/healthzentries now includestatus(incl.draining),sha256andbinary_versionper child — also groundwork for the §1.8 router-evacuation trackdevshardd (
devshard/cmd/devshardd/)GET /readyPOST /drainGET /drain/status{ready, draining, inflight}; inflight counts all non-lifecycle HTTP requests (incl. full SSE stream duration) and is exported as Prometheus gaugedevshardd_lifecycle_inflight_requestsDEVSHARD_SHUTDOWN_GRACEserver.Shutdownwindow (default10m); versiond's kill grace honors it (max(VERSIOND_DRAIN_KILL_GRACE, DEVSHARD_SHUTDOWN_GRACE))New configuration (all defaulted, no compose changes required)
VERSIOND_READY_PATH/readyVERSIOND_READY_TIMEOUT60sVERSIOND_DRAIN_PATH/drainVERSIOND_DRAIN_STATUS_PATH/drain/statusVERSIOND_DRAIN_TIMEOUT15mSIGTERMVERSIOND_DRAIN_POLL_INTERVAL1sVERSIOND_DRAIN_KILL_GRACE30sSIGTERMbeforeSIGKILLDEVSHARD_SHUTDOWN_GRACE10mSIGTERMTest infrastructure
VersiondTestsgains a same-version binary update scenario — a slowrequest keeps running on the old child through the swap, continuity probes assert no
routing gap, and the draining child is observed to exit.
local-test-net'stestapp-servernow builds per-name stamped binaries (-X main.Version=<slot>), asrequired by the protocol-embed check on this base branch.
DEVSHARD_CHAIN_GRPCtodevshardctl(itschain_restis deprecated on this basebranch), and
init-docker-genesis.shbinds the genesis node's gRPC to0.0.0.0:9090(join init already did this) so
edge-api/devshardctlcan reach it from peercontainers. Without these, devshard/edge-api testermint flows only pass on stale cached
images.
versioned/e2e):TestSameNameNewSHA_RollingUpdateDrainsOld— long requestsurvives a governance sha change while a concurrent request is served by the new binary;
testapp got
/ready,/drain,/drain/statusand graceful shutdown to act as adrain-aware child.
drain idle/timeout branches, GC retention, preflight matrix (stamped / legacy /
mismatch / timeout), oracle validation, port allocation.
Docs
rolling-update.md
updated to match the implementation: final env defaults, the legacy-compatibility
fallbacks, GC behavior, and the
WaitDelay/ shutdown-grace interaction.Compatibility with deployed nodes
--print-*, no/ready)DEVSHARD_BINARY_LOG_VERSIONunset → link-stamp fallback)No chain changes: versions still come from governance
DevshardEscrowParams.ApprovedVersionsvia the existing dapi
/versionsoracle; the wire contract is unchanged.Test plan
Unit (no Docker)
All green (also under
-race).Docker e2e
All scenarios pass, including
TestSameNameNewSHA_RollingUpdateDrainsOld: the in-flightslow request completes on the old binary, new requests are served by the new one, the
draining child exits cleanly.
Testermint
make local-build build-docker make run-tests TESTS="VersiondTests"Note: devshard/edge-api testermint flows require freshly built images plus the chain
gRPC fixes included here (
DEVSHARD_CHAIN_GRPC+ genesis0.0.0.0:9090bind); on stalecached images the pre-existing gaps are masked.
Out of scope / follow-ups
versiond-routerhost evacuation — upstreamdown+ reload, drainpoll, operator runbook. The
/healthzdraining visibility and/drain/statusadded hereare the signals that track will consume.
/healthzfor the §1.8 runbook.