Files
orca/cloud/packages/postgres-schema/src/apply-postgres-schema.ts
T
Jinwoo Hong 0699d73fd6 fix(relay): skip boot-time DDL when the catalog already has the object (#21147)
* fix(relay): skip boot-time DDL when the catalog already has the object

CREATE INDEX IF NOT EXISTS and ALTER TABLE ADD COLUMN IF NOT EXISTS take
their relation lock before the server evaluates the existence test, so a
boot on an already-migrated database still joins the lock queue. Relation
locks are granted in queue order, so every writer queues behind it.

The shared runner now asks pg_catalog whether the index or column is
already there and skips the statement when a row comes back, and 55P03
is no longer retried by default: with the pre-check ahead of it, a lock
timeout means the object is genuinely missing and each retry re-enters
the queue. Push keeps the old retry behind an explicit option.

* fix(relay): tie the index pre-check to its table and fail on an unreadable target

Three defects found in review of the auth reference implementation:

- The catalog query matched an index by name inside the table's namespace
  without checking it belonged to that table. Index names are unique per
  schema, not per table, so a same-named index on a sibling table answered
  yes and the real index was skipped forever. Added i.indrelid = t.oid.
- Lock-target derivation read a keyword sitting in an identifier position as
  the object name: CREATE UNIQUE INDEX CONCURRENTLY ON t(c) yielded the name
  CONCURRENTLY, and ADD COLUMN IF NOT EXISTS with no column yielded IF. A
  wrong target is worse than none, so keywords are now excluded and an index
  or column statement whose target cannot be read throws at boot with the
  statement text instead of falling through to the lock path.
- A concurrent-create collision retried the CREATE INDEX, taking SHARE on the
  table again for an object another director had just finished creating. The
  catalog is re-asked instead and a present object counts as skipped.

* fix(relay): pre-check constraint swaps so a warm boot sends no DDL at all

The two ALTER TABLE constraint statements were the last lock-taking
statements without a pre-check, so every boot still took ACCESS EXCLUSIVE
on relay_region_rehome_attempts twice.

A lock target now carries the catalog answer that means there is nothing
left to do. ADD CONSTRAINT skips when pg_constraint already names it; DROP
CONSTRAINT IF EXISTS is the inverse and skips when it does not, because
nothing to drop is nothing to do. The match is by name only: the CHECK body
is generated from RELAY_REGIONS, so comparing it would re-run the swap on
every region change. Changing a definition under the same name is an
operator migration, and the rule comment beside SCHEMA says so.

A bare DROP CONSTRAINT gets no target and throws at boot, because skipping
it would swallow the undefined_object the server is supposed to raise.

The census invariant is now that every lock-taking statement has a
pre-check, with no exceptions, and the warm-boot Postgres test asserts zero
statements sent rather than two.

* fix(relay): refuse a multi-action ALTER TABLE instead of pre-checking its first action

`ALTER TABLE t ADD COLUMN IF NOT EXISTS a TEXT, ADD COLUMN IF NOT EXISTS b
TEXT` derived the target for `a` alone, so once `a` existed the whole
statement was skipped and `b` was never added. The first subcommand parses,
so neither the parse throw nor the census caught it.

A lock-taking ALTER TABLE with a comma outside parentheses, quotes and
comments now throws at boot. One action per statement, or no pre-check is
possible. Commas inside a parenthesised type, a CHECK body, a quoted
default or a comment are unaffected, and push's 18 statements still parse.

* fix(relay): strip every comment before classifying, fold catalog names, count brackets

Four findings from the bot reviews on #21147:

- A comment between two keywords (ALTER TABLE t ADD /* note */ COLUMN c
  TEXT) was invisible to both the classification regexes and the must-parse
  shapes, so the statement got no target AND no throw and ran with no
  pre-check. Every comment is now stripped quote-aware before classification,
  nested block comments included. The server is still sent the original text.
- hasTopLevelComma counted parentheses but not square brackets, so
  ADD COLUMN c bigint[] DEFAULT ARRAY[1, 2] read as two subcommands and
  failed the boot.
- bareIdentifier split a qualified name on '.' regardless of quoting, so
  "a.b" became b", and it kept the written case while Postgres folds an
  unquoted identifier to lower case before storing it in relname, attname
  and conname. The name is now tokenised quote-aware and folded, with the
  qualified table text still passed to to_regclass as written.
- sqlWithoutLeadingComments is renamed sqlWithoutComments to match.

Relay's 74 statements and push's 18 all still parse, and no relay target
name changed: every identifier there was already lower case.

* fix(relay): treat a dollar-quoted body as opaque in both scanners

A comment marker, comma, parenthesis or bracket inside `$$...$$` or
`$tag$...$tag$` is text. The closing delimiter has to match the opening tag
exactly, so an inner `$$` inside a `$tag$` body is more text rather than the
end, and a tag cannot start with a digit, which keeps a `$1` placeholder
from reading as an opener.

Relay's pg_stat_statements DO block is the only dollar-quoted statement in
the schema, and it now survives the stripper byte-identical. A test asserts
that against the real statement.
2026-09-17 01:00:28 -04:00

180 lines
6.7 KiB
TypeScript

import { catalogObjectPresence, type SchemaCatalogQuery } from './catalog-object-precheck.js'
import {
requireSchemaLockTarget,
sqlWithoutComments,
type SchemaLockTarget
} from './schema-lock-target.js'
const RETRYABLE_SCHEMA_CODES = new Set(['57014'])
const LOCK_NOT_AVAILABLE = '55P03'
const DEFAULT_RETRY_DEADLINE_MS = 30_000
const RETRY_BASE_DELAY_MS = 250
const RETRY_MAX_DELAY_MS = 2_000
const DEFAULT_EVENT_PREFIX = 'orca_relay_postgres_schema'
export type SchemaStartupOptions = {
// Enables the catalog pre-check. Without it every lock-taking statement is sent as before.
catalogQuery?: SchemaCatalogQuery
eventPrefix?: string
now?: () => number
random?: () => number
retryDeadlineMs?: number
// Only for a caller with no catalog pre-check, where a lock timeout still says nothing about
// whether the object exists.
retryLockTimeout?: boolean
wait?: (delayMs: number) => Promise<void>
}
export type SchemaApplySummary = { ran: number; skipped: number }
function retryDelayMs(attempt: number, random: () => number): number {
const ceiling = Math.min(RETRY_BASE_DELAY_MS * 2 ** (attempt - 1), RETRY_MAX_DELAY_MS)
return Math.ceil(ceiling * (0.5 + random() * 0.5))
}
function wait(delayMs: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, delayMs))
}
const CREATE_TABLE_IF_NOT_EXISTS = /^CREATE\s+TABLE\s+IF\s+NOT\s+EXISTS\b/i
const CREATE_INDEX_IF_NOT_EXISTS = /^CREATE\s+(?:UNIQUE\s+)?INDEX\s+IF\s+NOT\s+EXISTS\b/i
const ALTER_TABLE_ADD_CONSTRAINT = /^ALTER\s+TABLE\s+\S+\s+ADD\s+CONSTRAINT\b/i
// `IF NOT EXISTS` only checks the name before the catalog inserts, so the loser of a concurrent
// CREATE can fail on the catalog unique index (23505) or, when the winner has already committed by
// the time the loser reaches TypeCreate/heap_create_with_catalog, on the name check those routines
// repeat (42710 duplicate type, 42P07 duplicate relation). Each is a no-op on the next attempt.
function concurrentCreateCollision(
value: { code?: unknown; constraint?: unknown },
sql: string
): boolean {
if (CREATE_TABLE_IF_NOT_EXISTS.test(sql)) {
return (
(value.code === '23505' && value.constraint === 'pg_type_typname_nsp_index') ||
value.code === '42710' ||
value.code === '42P07'
)
}
if (CREATE_INDEX_IF_NOT_EXISTS.test(sql)) {
return (
(value.code === '23505' && value.constraint === 'pg_class_relname_nsp_index') ||
value.code === '42P07'
)
}
return false
}
function constraintAlreadyApplied(error: unknown, sql: string): boolean {
return (
ALTER_TABLE_ADD_CONSTRAINT.test(sql) && (error as { code?: unknown } | null)?.code === '42710'
)
}
function retryableSchemaError(error: unknown, sql: string): boolean {
const value = (error as { code?: unknown; constraint?: unknown } | null) ?? {}
return RETRYABLE_SCHEMA_CODES.has(String(value.code)) || concurrentCreateCollision(value, sql)
}
// Evaluated immediately before each statement, so a pre-check still sees the objects the statements
// ahead of it created in this same boot.
async function nothingToDo(
target: SchemaLockTarget | undefined,
options: SchemaStartupOptions,
eventPrefix: string
): Promise<boolean> {
const catalogQuery = options.catalogQuery
if (!catalogQuery || !target) return false
const presence = await catalogObjectPresence(catalogQuery, target)
if (presence.present !== (target.skipWhen === 'present')) return false
console.log(
JSON.stringify({
event: `${eventPrefix}_object_${target.skipWhen}`,
kind: target.kind,
table: target.table,
name: target.name,
indisvalid: presence.indisvalid
})
)
return true
}
export async function applyPostgresSchema(
statements: string[],
query: (statement: string) => Promise<unknown>,
options: SchemaStartupOptions = {}
): Promise<SchemaApplySummary> {
const eventPrefix = options.eventPrefix ?? DEFAULT_EVENT_PREFIX
const now = options.now ?? Date.now
const random = options.random ?? Math.random
const pause = options.wait ?? wait
const deadlineAt = now() + (options.retryDeadlineMs ?? DEFAULT_RETRY_DEADLINE_MS)
const summary: SchemaApplySummary = { ran: 0, skipped: 0 }
for (const statement of statements) {
// Throws when an index or column statement's target cannot be read, rather than sending it
// unchecked into the lock queue.
const target = requireSchemaLockTarget(statement)
if (await nothingToDo(target, options, eventPrefix)) {
summary.skipped += 1
continue
}
const sql = sqlWithoutComments(statement)
let attempt = 1
while (true) {
try {
await query(statement)
summary.ran += 1
break
} catch (error) {
if (constraintAlreadyApplied(error, sql)) {
summary.skipped += 1
break
}
const code = String((error as { code?: unknown } | null)?.code)
// With the pre-check ahead of it a lock timeout means the object is genuinely missing and
// this boot lost the queue. Relation locks are granted in queue order, so each retry parks
// every writer behind it again for another timeout. Fail once, loudly.
if (code === LOCK_NOT_AVAILABLE && !options.retryLockTimeout) {
console.error(
JSON.stringify({
event: `${eventPrefix}_lock_timeout`,
code,
statement: sql.split('\n')[0],
detail: 'boot-time DDL could not take its lock; retrying would requeue every writer'
})
)
throw error
}
// The object was created between the pre-check and this statement. Re-asking the catalog
// is the cheap answer; retrying the CREATE INDEX would take SHARE on the table again for
// an object that is already there.
if (
concurrentCreateCollision((error as { code?: unknown; constraint?: unknown }) ?? {}, sql) &&
(await nothingToDo(target, options, eventPrefix))
) {
summary.skipped += 1
break
}
const remainingMs = deadlineAt - now()
const retryable =
retryableSchemaError(error, sql) ||
(code === LOCK_NOT_AVAILABLE && options.retryLockTimeout === true)
if (!retryable || remainingMs <= 0) {
if (retryable) {
console.warn(
JSON.stringify({ event: `${eventPrefix}_retry_exhausted`, code, attempts: attempt })
)
}
throw error
}
const delayMs = Math.min(remainingMs, retryDelayMs(attempt, random))
console.warn(JSON.stringify({ event: `${eventPrefix}_retry`, code, attempt, delayMs }))
await pause(delayMs)
attempt += 1
}
}
}
console.log(JSON.stringify({ event: `${eventPrefix}_applied`, ...summary }))
return summary
}