Files
orca/cloud/dev/scripts/prepare-relay-production-capacity-canary.test.mjs
Jinwoo Hong c8a5580659 fix(relay): re-place hosts off a cell isolated for a roll (#21911)
* fix(relay): re-place hosts off a cell isolated for a roll

A roll isolates a cell by moving it out of the 'general' admission class; the
cell then refuses every attach with 4503. The director never noticed, because
the only liveness test it applies to a host's current cell reads
`relay_cell_runtime.ready` and the heartbeat, and an isolated cell keeps
heartbeating ready=1 for the whole drain. So every host on that cell was handed
its own dead cell, closed, and handed it back — 500-1,900 hosts looping for
13-16 minutes per cell roll, at ~6 dials each per minute, with no neighbour
absorbing anything.

The sticky lane now treats a live incumbent whose admission is 'migration-only'
— the state a roll's isolate step writes — the same way it treats a dead one:
it returns null, which means "fall through to placement". The placement lane
had the identical hole eleven lines further down, so it takes the same
predicate; without that second swap the sticky change is inert, because
placement would hand the pin straight back (a draining cell has more headroom
than anyone). An isolated incumbent skips the dead-cell fence branch: that
branch exists to prove an unreachable cell stopped serving a host, and this one
is reachable and enforces the epoch itself.

'existing-only' is deliberately untouched — those cells serve the hosts they
already hold, and only `assignmentStrandedOnUnservedCell` may release that pin.
A host with an open `relay_assignment_migrations` row keeps its pin too, so
this stays disjoint from the migration machinery.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(relay): gate re-placement on a roll-isolation marker, not on admission

Review of the first commit found the predicate wrong. `migration-only` is an
admission class, not a drain signal: an Asia `--mode rollback`, an evacuation or
forward-recovery target awaiting a separate promote dispatch, a failed same-cap
wave's re-isolate, an abandoned migration retired on its target and a rehome
settlement all park loaded cells there durably, with no migration lease and no
open migration row. All five were indistinguishable from a roll's isolate, so
the first commit would have converted `operate-relay-asia-admission --mode
rollback` from a reversible admission flip into a mass move of ~4,000 hosts —
and, because `leastLoadedCell` treated region as a preference, into us-central1.

The signal is now an explicit stamp. `relay_cell_admission` gains a nullable
`roll_isolated_at`, added through the shared schema runner's catalog pre-check
so a migrated database takes no relation lock on boot and an un-migrated one
gets a catalog-only rewrite. The same-cap isolate step is its only writer, via a
new optional `rollIsolatedCells` on the selector apply; the same UPDATE that
writes the state clears the stamp whenever a cell leaves 'migration-only', so a
restore cannot leave one behind and a failed wave's re-isolate keeps the one it
has. Every other admission writer omits the field, so its cells stay unmarked
and their hosts stay pinned. Old directors ignore the field; old callers never
send it.

Region is now a constraint rather than a preference on this path only: a
re-placement must find a general, live cell with connection headroom in the
host's own region, or the pin is kept and one
`orca_relay_sticky_replacement_deferred` event is logged. Cross-region spill is
no longer reachable here.

The fence bypass is narrowed to a live incumbent. It was always a no-op for the
intended case, and for a stamped cell that stops heartbeating while still
holding sockets it reopened split-brain; that cell now takes the dead-cell path
unchanged.

Also: the hot-path admission reader no longer throws on an unrecognised state —
it sits on every sticky dial and the rule it feeds is "move the host", so an
unreadable row has to mean "don't". And the sticky lane reads the admission row
once for both the stranded rule and the stamp instead of twice.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(relay): emit the re-placement events after the transaction commits

CodeRabbit on assignment-store.ts:1075. Both events were written where they are
decided, which is inside assignOnce's transaction. A reservation or lease write
failing after that point rolls the placement back, but a line already on stdout
cannot be rolled back with it — so the canary this PR asks an operator to read
would count re-placements that never happened, and a Postgres transaction retry
could leave a stale line behind as well.

The transaction now returns its events alongside the RelayAssignment and the
caller flushes them once it has resolved. Returning them rather than setting a
variable in the enclosing scope is what makes the retry case safe too: only the
attempt that committed can carry its events out. assign()'s signature is
unchanged; the extra shape lives entirely inside assignOnce.

orca_relay_sticky_replacement_deferred was moved the same way. It cost one more
push into the array that already existed, and it is decided inside the same
transaction, so leaving it behind would have been the odd case rather than the
cheap one.

The new test injects a failure on the first write after the decision, asserts no
event is emitted, and asserts the assignment is still on its original cell —
without that second assertion the absence would only prove the emit was early,
not that it would have been wrong. A control dial with nothing injected emits
exactly one event, so the case cannot pass on a broken harness. With the emit
put back inside the transaction, it fails.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(relay): expire the roll stamp, correct the wire note, assert the stamp landed

Delta review findings B, D and E. A (the deferral path's cost) is deliberately
not implemented; it is now written up under Follow-ups in the PR body as
required before any Asia roll, because it cannot fire in a US canary.

B, which also closes C: the stamp was written, carried and never compared to
anything. A roll isolates and restores one cell inside ~15 minutes, so a stamp
older than two hours is not a roll in progress. It is a failed wave whose
failsafe re-isolated a possibly healthy cell and is waiting on an operator — the
postmortem in this tree records gaps of hours — or an orphan left by a director
rollback whose restore wrote 'general' without the clause that clears the stamp,
which the selector's 'keep' branch would then preserve until some later park
reactivated it. Both want the same answer and it is the pre-existing one: keep
the pin. One comparison against a value already on the row.

The bound takes the caller's `now` rather than reading the clock again, so one
assign reasons about one instant; the stamp's age is now a thing that decides
whether a host moves, and two clock reads could disagree across it.

D: the comment beside the new request field claimed an updated caller reaching
an older director "is simply ignored". The schema is .strict(), so it is a 400.
That fails closed — the isolate aborts before MUTATION_STARTED is set and
nothing is written — but it is a deploy ordering constraint, and it was
undocumented. The comment now says so and the PR body's rollout notes carry it.

E: nothing read the `rollIsolated` the script already prints, so an older script
against a newer director would silently produce today's behaviour and the canary
would read as "the fix did nothing" with no way to tell that from a wrong
premise. Both isolate steps now assert it, beside the generation they already
parse.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
2026-09-21 04:18:57 -04:00

353 lines
12 KiB
JavaScript

import assert from 'node:assert/strict'
import { describe, it } from 'node:test'
import {
parseProductionCapacityCellArguments,
prepareProductionCapacityCell,
PRODUCTION_CAPACITY_CELL_IDS
} from './prepare-relay-production-capacity-canary.mjs'
const config = {
directorOrigin: 'https://relay.onorca.dev',
cellOrigin: 'https://c26.relay.onorca.dev',
cellId: 'production-gce-c26'
}
const membership = {
existingOnly: ['production-gce-c1'],
migrationOnly: ['production-gce-c17'],
general: ['production-gce-c25', 'production-gce-c26']
}
function response(body, status = 200) {
return new Response(JSON.stringify(body), {
status,
headers: { 'content-type': 'application/json' }
})
}
function canaryFetch() {
let selector = { generation: 20, attemptId: null, membership }
const calls = []
const fetch = async (url, init) => {
const path = new URL(url).pathname
const body = JSON.parse(init.body)
calls.push({ path, body })
if (path === '/v1/admin/admission-selector/status') {
return response({
v: 1,
selector,
intent: body.attemptId
? {
attemptId: body.attemptId,
state: 'committed',
expectedGeneration: selector.generation - 1,
intendedGeneration: selector.generation,
membership: selector.membership
}
: null
})
}
if (path === '/v1/admin/admission-selector/apply') {
selector = {
generation: selector.generation + 1,
attemptId: body.attemptId,
membership: body.membership
}
return response({ v: 1, changed: true, selector })
}
if (path === '/v1/admin/drain') return response({ v: 1, draining: true })
throw new Error(`unexpected ${path}`)
}
return { calls, fetch, selector: () => selector }
}
describe('production Relay capacity cell admission', () => {
it('allows only the serving rollout cells', () => {
assert.deepEqual(PRODUCTION_CAPACITY_CELL_IDS, [
'production-gce-c7',
'production-gce-c8',
'production-gce-c9',
'production-gce-c10',
'production-gce-c13',
'production-gce-c14',
'production-gce-c15',
'production-gce-c16',
'production-gce-c19',
'production-gce-c20',
'production-gce-c21',
'production-gce-c22',
'production-gce-c23',
'production-gce-c24',
'production-gce-c25',
'production-gce-c26'
])
assert.deepEqual(parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c7.relay.onorca.dev',
'--cell-id', 'production-gce-c7',
'--mode', 'isolate'
]), {
directorOrigin: 'https://relay.onorca.dev',
cellOrigin: 'https://c7.relay.onorca.dev',
cellId: 'production-gce-c7',
mode: 'isolate',
paceWindowMs: 0
})
assert.throws(() => parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c17.relay.onorca.dev',
'--cell-id', 'production-gce-c17',
'--mode', 'isolate'
]), /not approved/)
assert.throws(() => parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c8.relay.onorca.dev',
'--cell-id', 'production-gce-c7',
'--mode', 'isolate'
]), /origin is not exact/)
assert.throws(() => parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c27.relay.onorca.dev',
'--cell-id', 'production-gce-c27',
'--mode', 'isolate'
]), /not approved/)
})
it('admits the same-cap Asia and migration-only cells only under the same-cap allowlist', () => {
for (const cellId of [
'production-gce-c27', 'production-gce-c28', 'production-gce-c29',
// Migration-only canaries: the US-only capacity rollout never touches them either.
'production-gce-c17', 'production-gce-c18'
]) {
const hostname = cellId.slice('production-gce-'.length)
assert.deepEqual(parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', `https://${hostname}.relay.onorca.dev`,
'--cell-id', cellId,
'--approved-cells', 'same-cap',
'--mode', 'isolate'
]), {
directorOrigin: 'https://relay.onorca.dev',
cellOrigin: `https://${hostname}.relay.onorca.dev`,
cellId,
mode: 'isolate',
paceWindowMs: 0
})
}
for (const cellId of ['production-gce-c12', 'production-gce-c30']) {
const hostname = cellId.slice('production-gce-'.length)
assert.throws(() => parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', `https://${hostname}.relay.onorca.dev`,
'--cell-id', cellId,
'--approved-cells', 'same-cap',
'--mode', 'isolate'
]), /not approved/)
}
assert.throws(() => parseProductionCapacityCellArguments([
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c27.relay.onorca.dev',
'--cell-id', 'production-gce-c27',
'--approved-cells', 'every-cell',
'--mode', 'isolate'
]), /not a known allowlist/)
})
it('isolates only the selected cell without depending on its runtime', async () => {
const fake = canaryFetch()
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'isolate' },
{ fetch: fake.fetch, token: 'token' }
)
assert.equal(result.admissionState, 'migration-only')
assert.deepEqual(fake.selector().membership, {
existingOnly: ['production-gce-c1'],
migrationOnly: ['production-gce-c17', 'production-gce-c26'],
general: ['production-gce-c25']
})
assert.doesNotMatch(fake.calls.map(({ path }) => path).join(','), /\/v1\/admin\/drain/)
})
it('stamps the roll isolation on isolate and never on activate', async () => {
// The stamp is what lets the director tell a cell parked for a restart from
// an evacuation target or an Asia rollback, both of which must keep their
// hosts. Only this call site may send it.
const isolate = canaryFetch()
await prepareProductionCapacityCell(
{ ...config, mode: 'isolate' },
{ fetch: isolate.fetch, token: 'token' }
)
const isolateApply = isolate.calls.find(
({ path }) => path === '/v1/admin/admission-selector/apply'
)
assert.deepEqual(isolateApply.body.rollIsolatedCells, [config.cellId])
// Restore has to be a real apply, not the no-op an already-general cell
// takes, or the assertion below proves nothing.
isolate.calls.length = 0
const restored = await prepareProductionCapacityCell(
{ ...config, mode: 'activate' },
{ fetch: isolate.fetch, token: 'token' }
)
assert.equal(restored.admissionState, 'general')
const restoreApply = isolate.calls.find(
({ path }) => path === '/v1/admin/admission-selector/apply'
)
assert.ok(restoreApply, 'restore must issue an apply')
assert.equal(restoreApply.body.rollIsolatedCells, undefined)
assert.ok(restoreApply.body.membership.general.includes(config.cellId))
})
it('drains the selected cell independently after durable isolation', async () => {
const fake = canaryFetch()
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'drain' },
{ fetch: fake.fetch, token: 'token' }
)
assert.deepEqual(result, { changed: false, drained: true, paceWindowMs: 0 })
assert.deepEqual(fake.calls, [{
path: '/v1/admin/drain',
body: { v: 1, graceMs: 0 }
}])
})
it('paces the drain send when the roll asks for a window', async () => {
const fake = canaryFetch()
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'drain', paceWindowMs: 120_000 },
{ fetch: fake.fetch, token: 'token' }
)
assert.deepEqual(result, { changed: false, drained: true, paceWindowMs: 120_000 })
assert.deepEqual(fake.calls, [{
path: '/v1/admin/drain',
body: { v: 1, graceMs: 0, paceWindowMs: 120_000 }
}])
})
it('drains unpaced when the cell image rejects the pacing field', async () => {
const bodies = []
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'drain', paceWindowMs: 120_000 },
{
token: 'token',
wait: async () => {},
fetch: async (url, init) => {
assert.equal(new URL(url).pathname, '/v1/admin/drain')
const body = JSON.parse(init.body)
bodies.push(body)
if (body.paceWindowMs !== undefined) return response({ error: 'invalid_request' }, 400)
return response({ v: 1, draining: true })
}
}
)
assert.deepEqual(result, { changed: false, drained: true, paceWindowMs: 0 })
assert.deepEqual(bodies, [
{ v: 1, graceMs: 0, paceWindowMs: 120_000 },
{ v: 1, graceMs: 0 }
])
})
it('fails a paced drain that the cell rejects for any other reason', async () => {
await assert.rejects(
prepareProductionCapacityCell(
{ ...config, mode: 'drain', paceWindowMs: 120_000 },
{
token: 'token',
wait: async () => {},
fetch: async () => response({ error: 'invalid_token' }, 401)
}
),
/returned 401/
)
})
it('refuses a pacing window that is not a bounded integer', () => {
const argv = (value) => [
'--director-origin', 'https://relay.onorca.dev',
'--cell-origin', 'https://c26.relay.onorca.dev',
'--cell-id', 'production-gce-c26',
'--mode', 'drain',
'--pace-window-ms', value
]
for (const value of ['-1', '300001', '1.5', 'soon']) {
assert.throws(() => parseProductionCapacityCellArguments(argv(value)), /pace-window-ms/)
}
assert.equal(parseProductionCapacityCellArguments(argv('300000')).paceWindowMs, 300_000)
})
it('restores only the selected cell to general admission', async () => {
const fake = canaryFetch()
await prepareProductionCapacityCell(
{ ...config, mode: 'isolate' },
{ fetch: fake.fetch, token: 'token' }
)
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'activate' },
{ fetch: fake.fetch, token: 'token' }
)
assert.equal(result.admissionState, 'general')
assert.deepEqual(fake.selector().membership, membership)
})
it('refuses an irreversible existing-only target', async () => {
const fetch = async () => response({
v: 1,
selector: {
generation: 20,
attemptId: null,
membership: {
existingOnly: ['production-gce-c26'],
migrationOnly: ['production-gce-c17'],
general: ['production-gce-c25']
}
},
intent: null
})
await assert.rejects(
prepareProductionCapacityCell(
{ ...config, mode: 'isolate' },
{ fetch, token: 'token' }
),
/irreversible/
)
})
it('retries a transient 503 on the cell drain endpoint', async () => {
let calls = 0
const result = await prepareProductionCapacityCell(
{ ...config, mode: 'drain' },
{
token: 'token',
wait: async () => {},
fetch: async (url) => {
assert.equal(new URL(url).pathname, '/v1/admin/drain')
calls += 1
if (calls === 1) return response({ error: 'warming up' }, 503)
return response({ v: 1, draining: true })
}
}
)
assert.equal(calls, 2)
assert.deepEqual(result, { changed: false, drained: true, paceWindowMs: 0 })
})
it('fails when both drain attempts return a transient 503', async () => {
let calls = 0
await assert.rejects(
prepareProductionCapacityCell(
{ ...config, mode: 'drain' },
{
token: 'token',
wait: async () => {},
fetch: async () => {
calls += 1
return response({ error: 'warming up' }, 503)
}
}
),
/returned 503/
)
assert.equal(calls, 2)
})
})