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
2026-10-31 00:57:45 +00:00 · 2026-10-14 15:35:31 -04:00
parent 17959a90f7
commit 68b186e2aa
8 changed files with 66 additions and 44 deletions
@@ -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])
@@ -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
@@ -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,
@@ -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,