From 68b186e2aa45916234559209e5444a564d6de0ce Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:35:31 -0400 Subject: [PATCH] fix(relay): end the restart-safe quiet at the drain's pace window, not a window after it (#28677) * fix(relay): end the restart-safe quiet at the drain's pace window, not a window after it A paced drain tells its last host when the window ends, so the restart-safe wait has to see an empty runtime reaching past that point. It counted a full window of quiet samples from the first empty one instead, and the runtime only empties as the window ends, so every same-cap cell waited the window twice: about 25 minutes per Asia cell at 1200 s on 2026-10-13, and 5 minutes per 300 s cell. The window is now measured from the verifier's own start, which runs after the cell accepted the drain and so ends no earlier than the cell's own window. A pass still needs two consecutive restart-safe samples, the last taken at or after that end, and any non-zero runtime counter or open migration still restarts the count. Progress lines report paceWindowRemainingMs in place of the old sample target. Claude-Session: f2fe8cb5-4467-4a25-8aab-98598b70791f * fix(relay): drop the same-cap drain timeout's double-wait allowance and stale quiet wording The drain step's timeout added the window's excess over 300000 only because restart-safe used to wait a second window after the cell emptied, so it is now the 15-minute migration lease plus the window for every pace (35 minutes at 1200000, unchanged at 300000 and below). Docs, workflow comments and the shadow gate's settledAfterSeconds note now describe the window-end rule. The always-quiet test starts its clock late, so a window measured from zero fails it, and the overlapping late-control test is folded away. Claude-Session: f2fe8cb5-4467-4a25-8aab-98598b70791f * docs(relay): state the restart-safe window-end rule exactly Claude-Session: f2fe8cb5-4467-4a25-8aab-98598b70791f * docs(relay): the slowest restart-safe wait is now 35 minutes Claude-Session: f2fe8cb5-4467-4a25-8aab-98598b70791f --- ...d-deploy-relay-production-capacity-job.yml | 4 +- ...d-deploy-relay-production-same-cap-job.yml | 9 ++-- .../cloud-prove-relay-staging-capacity.yml | 4 +- .../relay-production-same-cap-wave.test.mjs | 8 +--- .../scripts/relay-same-cap-shadow-gate.mjs | 4 +- .../verify-relay-capacity-transition.mjs | 20 +++++---- .../verify-relay-capacity-transition.test.mjs | 45 ++++++++++++++----- cloud/docs/relay-workflows.md | 16 +++---- 8 files changed, 66 insertions(+), 44 deletions(-) diff --git a/.github/workflows/cloud-deploy-relay-production-capacity-job.yml b/.github/workflows/cloud-deploy-relay-production-capacity-job.yml index dbb44e5f372..728e8eabb65 100644 --- a/.github/workflows/cloud-deploy-relay-production-capacity-job.yml +++ b/.github/workflows/cloud-deploy-relay-production-capacity-job.yml @@ -47,8 +47,8 @@ jobs: timeout-minutes: 75 environment: production env: - # These drains are unpaced, but restart-safe still needs a quiet runtime for the production - # pace window, the same one the same-cap drain uses. + # These drains are unpaced, but restart-safe still waits out the production pace window + # from the drain, the same one the same-cap drain uses. RESTART_SAFE_PACE_WINDOW_MS: '300000' GCP_PROJECT_ID: onorca-cloud GCP_REGION: ${{ vars.PRODUCTION_GCP_REGION }} diff --git a/.github/workflows/cloud-deploy-relay-production-same-cap-job.yml b/.github/workflows/cloud-deploy-relay-production-same-cap-job.yml index 0c973641150..8d659ce3349 100644 --- a/.github/workflows/cloud-deploy-relay-production-same-cap-job.yml +++ b/.github/workflows/cloud-deploy-relay-production-same-cap-job.yml @@ -54,8 +54,7 @@ jobs: # /v1/assign at hosts / window and is re-placed through the director's drain-return # lane. The wave validator admits only its closed set, and anything below the 300000 # default only on US general cells: Asia drains are bound by their targets' accept - # rate. The transition wait below is the 15-min migration lease plus this window, plus - # its excess over 300000, since restart-safe waits a window of quiet after the drain. A + # rate. The transition wait below is the 15-min migration lease plus this window. A # cell on an older image drains at once, or refuses a window above 300000. DRAIN_PACE_WINDOW_MS: ${{ inputs.drain-pace-window-ms }} steps: @@ -504,7 +503,7 @@ jobs: run: echo 'Exact same-cap rollback point verified.' # The sample can age the first one-hour token by up to 20 min; the drain's restart-safe - # wait (20 min at the default pace, 50 at the slowest) gets a fresh one. + # wait (20 min at the default pace, 35 at the slowest) gets a fresh one. - id: drain-auth if: ${{ inputs.mode != 'verify' && env.ROLLBACK_RESUME != 'true' }} uses: google-github-actions/auth@v2 @@ -555,7 +554,7 @@ jobs: # What the cell accepted, which is 0 when an older image fell back to an unpaced drain. echo "drain-applied-pace-window-ms=$(jq -er '.paceWindowMs' <<< "${DRAIN_RESULT}")" \ >> "${GITHUB_OUTPUT}" - # Restart-safe means an empty runtime for a whole pace window after the drain; leases + # Restart-safe means an empty runtime that lasts past the drain's pace window; leases # held by unplaceable hosts are reported, not waited on (c28, 2026-10-01). node dev/scripts/verify-relay-capacity-transition.mjs \ --director-origin "${DIRECTOR_ORIGIN}" --cell-origin "${CELL_ORIGIN}" \ @@ -564,7 +563,7 @@ jobs: --heartbeat either --admission migration-only --draining required \ --activity restart-safe --expected-image-digests "${PREDECESSOR_IMAGE_DIGEST}" \ --pace-window-ms "${DRAIN_PACE_WINDOW_MS}" \ - --timeout-ms "$((900000 + DRAIN_PACE_WINDOW_MS + (DRAIN_PACE_WINDOW_MS > 300000 ? DRAIN_PACE_WINDOW_MS - 300000 : 0)))" + --timeout-ms "$((900000 + DRAIN_PACE_WINDOW_MS))" echo "drain-settled-at=$(date -u +%FT%TZ)" >> "${GITHUB_OUTPUT}" - id: capacity-auth diff --git a/.github/workflows/cloud-prove-relay-staging-capacity.yml b/.github/workflows/cloud-prove-relay-staging-capacity.yml index 8ee7116b29e..d841c3f397d 100644 --- a/.github/workflows/cloud-prove-relay-staging-capacity.yml +++ b/.github/workflows/cloud-prove-relay-staging-capacity.yml @@ -69,8 +69,8 @@ jobs: DIRECTOR_SERVICE_NAME: orca-cloud-relay-staging DIRECTOR_ORIGIN: https://relay-staging.onorca.dev CELL_ORIGIN: https://c3.relay-staging.onorca.dev - # These drains are unpaced, but restart-safe still needs a quiet runtime for the production - # pace window, the same one the same-cap drain uses. + # These drains are unpaced, but restart-safe still waits out the production pace window + # from the drain, the same one the same-cap drain uses. RESTART_SAFE_PACE_WINDOW_MS: '300000' TARGET_CELL_ID: staging-gce-c3 FALLBACK_CELL_ORIGIN: https://c2.relay-staging.onorca.dev diff --git a/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs b/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs index d679ae3c7e4..3e7ae952a47 100644 --- a/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs +++ b/cloud/dev/scripts/relay-production-same-cap-wave.test.mjs @@ -922,18 +922,14 @@ test('the workflows offer exactly the closed set and scale the drain wait with i assert.match(job, /\n DRAIN_PACE_WINDOW_MS: \$\{\{ inputs\.drain-pace-window-ms \}\}\n/) assert.doesNotMatch(job, /DRAIN_PACE_WINDOW_MS: '/) assert.match(job, /cell-class \\\n\s+--cell-id "\$\{TARGET_CELL_ID\}" --drain-pace-window-ms "\$\{DRAIN_PACE_WINDOW_MS\}"/) - // The 15-min migration lease plus the window, plus its excess over the default: restart-safe - // waits a whole window of quiet after the last send, so a slow drain needs about twice it. + // The 15-min migration lease plus the window: restart-safe passes once the window has ended. const timeout = /--timeout-ms "\$\(\((.+)\)\)"\n/.exec(job)[1] const timeoutMs = (pace) => Number(execFileSync( 'bash', ['-c', `DRAIN_PACE_WINDOW_MS=${pace}; echo $((${timeout}))`], { encoding: 'utf8' } )) - // Exactly the timeouts every pace had before the slower ones existed. - for (const pace of [300_000, 60_000, 30_000]) assert.equal(timeoutMs(pace), 900_000 + pace) for (const pace of SAME_CAP_DRAIN_PACE_WINDOWS_MS) { - assert.ok(timeoutMs(pace) >= 2 * pace + 600_000, String(pace)) + assert.equal(timeoutMs(pace), 900_000 + pace, String(pace)) } - assert.equal(timeoutMs(1_200_000), 3_000_000) const slowest = timeoutMs(Math.max(...SAME_CAP_DRAIN_PACE_WINDOWS_MS)) // The rest of the job keeps the 70 min it had beside the old 20-min drain step in 90. const jobMinutes = Number(/\n rollout:\n(?: [^\n]*\n)*? timeout-minutes: (\d+)\n/.exec(job)[1]) diff --git a/cloud/dev/scripts/relay-same-cap-shadow-gate.mjs b/cloud/dev/scripts/relay-same-cap-shadow-gate.mjs index 9332017f39a..cacfd7b7bda 100644 --- a/cloud/dev/scripts/relay-same-cap-shadow-gate.mjs +++ b/cloud/dev/scripts/relay-same-cap-shadow-gate.mjs @@ -419,8 +419,8 @@ export async function evaluateShadowGate(config, { appliedPaceWindowMs: config.drainAppliedPaceWindowMs, targetHosts: config.targetHosts, settledAt: config.drainSettledAt || null, - // Isolate to restart-safe, which includes the quiet the restart wait holds after the cell - // empties: (ceil(pace / 5 s) + 1) samples of 5 s. + // Isolate to restart-safe: the later of the pace window's end and the cell emptying, plus + // one 5 s sample. settledAfterSeconds: config.drainStartedAt && config.drainSettledAt ? Math.round((Date.parse(config.drainSettledAt) - Date.parse(config.drainStartedAt)) / 1000) : null diff --git a/cloud/dev/scripts/verify-relay-capacity-transition.mjs b/cloud/dev/scripts/verify-relay-capacity-transition.mjs index 0c819adc9b6..9e0ce7a71ac 100644 --- a/cloud/dev/scripts/verify-relay-capacity-transition.mjs +++ b/cloud/dev/scripts/verify-relay-capacity-transition.mjs @@ -420,14 +420,17 @@ export async function verifyCapacityTransition(config, overrides = {}) { if (health.ok !== true || health.connectionCapacityProtocol !== CAPACITY_PROTOCOL) { throw new Error('director is not capacity-protocol compatible') } - const deadline = now() + config.timeoutMs - // A paced drain can refill the runtime until its window ends, so the quiet must outlast it. - // Each poll waits a full interval, so consecutive samples bound the quiet time from below. - const requiredRestartSafeSamples = - Math.max(Math.ceil((config.paceWindowMs ?? 0) / POLL_INTERVAL_MS) + 1, 2) + const startedAt = now() + const deadline = startedAt + config.timeoutMs + // A paced drain can refill the runtime until its window ends, so the second of two consecutive + // quiet samples must be taken at or after that end. This runs after the cell accepted the drain, + // so the cell's window ends no later than this one. + const paceWindowEndsAt = startedAt + (config.paceWindowMs ?? 0) let restartSafeSamples = 0 let lastObservation = { runtimeAvailable: false } for (;;) { + // Before the read, so a sample taken inside the window cannot pass. + const sampledAt = now() const runtime = await cellRuntime(fetchImpl, config, token) lastObservation = { runtimeAvailable: runtime !== null } if ((runtime === null) === (config.runtime === 'unavailable')) { @@ -469,7 +472,7 @@ export async function verifyCapacityTransition(config, overrides = {}) { if ( matches && (config.activity !== 'restart-safe' || - restartSafeSamples + 1 >= requiredRestartSafeSamples) + (restartSafeSamples >= 1 && sampledAt >= paceWindowEndsAt)) ) { return { cellId: status.cellId, @@ -491,15 +494,16 @@ export async function verifyCapacityTransition(config, overrides = {}) { restartSafeSamples = 0 } if (config.activity === 'restart-safe') { + const paceWindowRemainingMs = Math.max(paceWindowEndsAt - sampledAt, 0) lastObservation = { ...lastObservation, restartSafeSamples, - requiredRestartSafeSamples + paceWindowRemainingMs } progress({ event: 'relay_capacity_transition_restart_progress', restartSafeSamples, - requiredRestartSafeSamples, + paceWindowRemainingMs, totalConnections: lastObservation.runtime?.totalConnections ?? null, preAuthConnections: lastObservation.runtime?.preAuthConnections ?? null, runtimeAvailable: lastObservation.runtimeAvailable, diff --git a/cloud/dev/scripts/verify-relay-capacity-transition.test.mjs b/cloud/dev/scripts/verify-relay-capacity-transition.test.mjs index 71d8d88b6a8..8d861b3bd57 100644 --- a/cloud/dev/scripts/verify-relay-capacity-transition.test.mjs +++ b/cloud/dev/scripts/verify-relay-capacity-transition.test.mjs @@ -856,8 +856,7 @@ test('restart timeout reports one of two safe settling samples', async () => { { fetch: base, token: 'masked-token' } ), (error) => { - assert.match(error.message, /"restartSafeSamples":1/) - assert.match(error.message, /"requiredRestartSafeSamples":2/) + assert.match(error.message, /"restartSafeSamples":1,"paceWindowRemainingMs":0/) return true } ) @@ -1177,8 +1176,8 @@ const pacedRestartConfig = { timeoutMs: 1_200_000 } -function fakeClock() { - let now = 0 +function fakeClock(start = 0) { + let now = start return { now: () => now, wait: async (milliseconds) => { @@ -1231,7 +1230,8 @@ test('passes after one pace window of an empty runtime and reports stranded leas }, token: 'masked-token', progress: (line) => lines.push(line), - ...fakeClock() + // A late start catches a window measured from zero instead of from the verifier's start. + ...fakeClock(1_000_000) }) // One sample at each end of a full five-minute pace window. assert.equal(runtimeReads, 61) @@ -1246,12 +1246,15 @@ test('passes after one pace window of an empty runtime and reports stranded leas assert.equal(lines.length, 60) assert.ok(lines.every((line) => line.event === 'relay_capacity_transition_restart_progress' && - line.requiredRestartSafeSamples === 61 && line.stranded.restartBlockingActivityLeases === 4)) assert.deepEqual(lines.slice(0, 2).map((line) => line.restartSafeSamples), [1, 2]) + assert.deepEqual( + lines.slice(0, 2).map((line) => line.paceWindowRemainingMs), + [300_000, 295_000] + ) }) -test('the pace window resets on any non-zero runtime sample', async () => { +test('a non-zero runtime sample restarts the quiet count', async () => { const quiet = harness(strandedCell) const busy = harness({ ...strandedCell, controls: 1 }) let runtimeReads = 0 @@ -1265,17 +1268,37 @@ test('the pace window resets on any non-zero runtime sample', async () => { progress: (line) => lines.push(line), ...fakeClock() }) - assert.equal(runtimeReads, 91) + // A mid-window reset adds no time; the quiet after it still reaches the window's end. + assert.equal(runtimeReads, 61) assert.deepEqual(lines.slice(28, 31).map((line) => line.restartSafeSamples), [29, 0, 1]) await assert.rejects( verifyCapacityTransition( { ...pacedRestartConfig, timeoutMs: 295_000 }, { fetch: quiet, token: 'masked-token', progress: () => {}, ...fakeClock() } ), - /"restartSafeSamples":60,"requiredRestartSafeSamples":61/ + /"restartSafeSamples":60,"paceWindowRemainingMs":5000/ ) }) +// c27 on 2026-10-13: the 20-minute paced drain emptied the runtime as its window closed, and the +// old gate then waited a second full window of quiet samples before the restart. +test('a runtime that empties as the pace window ends passes after two quiet samples', async () => { + let runtimeReads = 0 + const lines = [] + await verifyCapacityTransition(pacedRestartConfig, { + fetch: async (url, options) => { + if (new URL(url).pathname === '/v1/admin/runtime-status') runtimeReads += 1 + return await harness({ ...strandedCell, controls: runtimeReads <= 60 ? 1 : 0 })(url, options) + }, + token: 'masked-token', + progress: (line) => lines.push(line), + ...fakeClock() + }) + // Busy through 295 s; quiet at 300 s and 305 s, the first eligible pair. + assert.equal(runtimeReads, 62) + assert.deepEqual(lines.slice(59).map((line) => line.restartSafeSamples), [0, 1]) +}) + test('restart-safe fails on live runtime work or an open migration regardless of leases', async (t) => { for (const blocker of [ { controls: 1 }, @@ -1299,7 +1322,7 @@ test('restart-safe fails on live runtime work or an open migration regardless of } }) -test('unauthenticated redials do not reset the pace window', async () => { +test('unauthenticated redials do not restart the quiet count', async () => { let runtimeReads = 0 const lines = [] const result = await verifyCapacityTransition(pacedRestartConfig, { @@ -1439,7 +1462,7 @@ test('restart progress prints every counter the gate reads', async () => { assert.deepEqual(lines[0], { event: 'relay_capacity_transition_restart_progress', restartSafeSamples: 0, - requiredRestartSafeSamples: 61, + paceWindowRemainingMs: 300_000, totalConnections: 7, preAuthConnections: 1, runtimeAvailable: true, diff --git a/cloud/docs/relay-workflows.md b/cloud/docs/relay-workflows.md index 66b56b1e901..28ea41dc79e 100644 --- a/cloud/docs/relay-workflows.md +++ b/cloud/docs/relay-workflows.md @@ -461,8 +461,9 @@ margin covers. Each cell is isolated, drained until restart-safe, replaced from a targeted saved plan, and restored only after a new incarnation reports the exact digest, cap, heartbeat, and rehome protocol. Restart-safe means the cell runtime itself carries nothing live (no controls, in-flight connections, reserved connection units, splices, or queued bytes) and no -migration is open, for a whole drain pace window, which every live restart-safe call must pass. -Pre-auth and total connections are printed but do not reset the window: they include +migration is open, on two consecutive samples the second of which is taken at or after the end +of the drain pace window. Every live restart-safe call supplies that window (`--pace-window-ms`). +Pre-auth and total connections are printed but do not restart the count: they include unauthenticated redials that lose nothing on a restart. Director activity leases left by hosts that already went or cannot be placed do not hold the restart; every `relay_capacity_transition_restart_progress` sample and the final verified line report them under `stranded`. The durable @@ -539,11 +540,10 @@ drained. Then read the cell's live runtime image from The `drain-pace-window-ms` input sets the window the cell spreads its drain sends over. Each drained host re-dials the director as soon as it reads `drain`, so the window sets the re-placement arrival rate: hosts / window, about 1.8-2.7 hosts/s for a US cell at the default. The window also -sets two waits: restart-safe needs an empty runtime for (ceil(window / 5 s) + 1) consecutive -5-second samples, and the drain step's overall timeout is the 15-minute migration lease plus the -window, plus the window's excess over `300000` (20 minutes at the default, 50 at `1200000`). -Because the quiet starts only once the last host has left, a drain step takes about twice the -window: about 30 minutes at `900000` and 40 at `1200000`. +sets two waits: restart-safe needs an empty runtime on two consecutive 5-second samples, the +second at or after the window's end, and the drain step's overall timeout is the 15-minute +migration lease plus the window (20 minutes at the default, 35 at `1200000`). The runtime empties +as the window closes, so a drain step takes about the window plus a few seconds to half a minute. - Allowed values are `300000` (the default, and every wave before this input), `60000`, `30000`, `900000`, and `1200000`. The wave validator refuses anything else, and each cell job @@ -634,7 +634,7 @@ monitor already excludes 503s from its director 5xx rule. - its `paceVerdict` is PASS, sealed only from a report on that cell that drained at least 400 hosts at that pace; - time to empty is within window + hosts / 50 s + 10 s. Take `settledAfterSeconds` minus the - restart-safe quiet; this is an upper bound, since it includes the isolate. + one 5-second confirming sample; this is an upper bound, since it includes the isolate. The batch check enforces the PASS. A canary that sealed anything else authorizes only `300000` batches. The 400-host floor exists because a pace is an arrival rate (hosts /