mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 17:17:38 +00:00
* perf(relay): stop indexing the column every control renewal writes relay_assignment_activity_expiry indexes expires_at on relay_assignment_activity_leases, and expires_at is what every control renewal updates: ~471 calls/s, all of them non-HOT because a changed indexed column forbids HOT. The index has one reader, the 30s expiry sweep, which seq-scans the whole 14.8k-row table in under a millisecond. Drop it, and set fillfactor to 70 so a renewal has room for a second row version on its own page. Measured on postgres:16-alpine over 14.8k rows, WAL bytes per renewal and HOT ratio: index, fillfactor 100 (today) 0% HOT 371 B index, fillfactor 70 0% HOT 246 B no index, fillfactor 100 0.5% HOT 298 B no index, fillfactor 70 100% HOT 80 B Both are needed: the index makes HOT illegal, and the default fillfactor leaves no page space to make it possible. Neither statement can use the catalog pre-check as it stood. DROP INDEX IF EXISTS resolves the name before it locks, so once the index is gone it costs a catalog miss and takes no lock on the table - pinned in the lock-target census as the one exempt statement. ALTER TABLE SET does take a lock, so it gets a new 'reloption' target kind that asks pg_class.reloptions for the name=value pair, keeping the invariant that no lock-taking statement reaches a warm boot unchecked. * fix(relay): pre-check the activity-expiry drop and let it defer on a lock timeout The drop had no catalog pre-check, so it was sent on every boot, and a 55P03 from it was fatal: apply-postgres-schema throws on a lock timeout with no retry. On the migration boot that combination is a crash loop. All 28 directors reach the same DROP INDEX at once, it needs ACCESS EXCLUSIVE on a table written ~475/s with lock_timeout at 1s, and a boot that fails restarts the instance to re-queue the same DDL behind the same writers. Two changes: - A new 'index-by-name' lock target. A DROP INDEX names no table, so the existing index check could not serve it; this one resolves by name through the search_path with relkind = 'i', which is how the DROP itself resolves, and skips when absent. DROP INDEX now counts as lock-taking in the census, so it is covered rather than exempt, and IF EXISTS is required the way it is on DROP CONSTRAINT. - A 'schema-deferrable' marker, read from a statement's leading comment. A 55P03 on a marked statement logs orca_relay_postgres_schema_object_deferred and leaves the statement unapplied instead of failing the boot; the next boot re-sends it. Both activity-lease migrations carry it. Everything else keeps the old contract and still fails loudly. SchemaApplySummary gains a deferred count so a boot that skipped work is distinguishable from one with nothing to do. Verified against a real server: with the index present and the table held in ACCESS EXCLUSIVE by another session, both statements defer, the boot completes, nothing is half-applied, and the next boot finishes the job. A warm boot now sends neither statement at all.
306 lines
13 KiB
TypeScript
306 lines
13 KiB
TypeScript
// The object a boot-time DDL statement locks on Postgres, so the catalog can be asked whether it
|
|
// already exists before the statement joins the lock queue. `table` is kept exactly as the
|
|
// statement wrote it, schema qualification and quoting included, because it is fed to
|
|
// `to_regclass`; `name` is the bare identifier the catalog stores in `relname`/`attname`.
|
|
export type SchemaLockTarget =
|
|
| {
|
|
kind: 'index' | 'column' | 'constraint' | 'reloption'
|
|
table: string
|
|
name: string
|
|
// The catalog answer that means this statement has nothing left to do. Creating statements
|
|
// skip on present; `DROP CONSTRAINT IF EXISTS` is the inverse, because nothing to drop is
|
|
// done.
|
|
skipWhen: 'present' | 'absent'
|
|
}
|
|
// A `DROP INDEX` names no table, and needs none: an index name that resolves to nothing is
|
|
// nothing to drop, whatever table it used to belong to. Resolution is by name through the
|
|
// search_path, which is how the DROP itself would resolve it.
|
|
| { kind: 'index-by-name'; name: string; skipWhen: 'absent' }
|
|
|
|
// Keywords that sit in an identifier position when the optional clause before them is absent.
|
|
// Without this, `CREATE UNIQUE INDEX CONCURRENTLY ON t(c)` reads CONCURRENTLY as the index name and
|
|
// `ADD COLUMN IF NOT EXISTS` with no column reads IF as the column: a silently wrong target, which
|
|
// is worse than no target. Excluding them makes both throw instead. A column genuinely named `if`
|
|
// has to be quoted to be derivable, which is the safe direction to fail in.
|
|
const NOT_KEYWORD = '(?!(?:CONCURRENTLY|IF|NOT|EXISTS|ON|ONLY)\\b)'
|
|
const IDENTIFIER = `"(?:[^"]|"")*"|${NOT_KEYWORD}[A-Za-z_][A-Za-z0-9_$]*`
|
|
const QUALIFIED = `((?:${IDENTIFIER})(?:\\.(?:${IDENTIFIER}))?)`
|
|
|
|
// `$$...$$` and `$tag$...$tag$` are opaque: a comment marker, comma, parenthesis or bracket inside
|
|
// one is text. The closing delimiter must match the opening tag exactly, so an inner `$$` inside a
|
|
// `$tag$` body is more text rather than the end. The tag cannot start with a digit, which is what
|
|
// keeps a `$1` placeholder from reading as an opener.
|
|
const DOLLAR_QUOTE = /\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$/y
|
|
|
|
function dollarQuoteEnd(sql: string, index: number): number | undefined {
|
|
DOLLAR_QUOTE.lastIndex = index
|
|
const opener = DOLLAR_QUOTE.exec(sql)?.[0]
|
|
if (opener === undefined) return undefined
|
|
const close = sql.indexOf(opener, index + opener.length)
|
|
return close === -1 ? sql.length : close + opener.length
|
|
}
|
|
|
|
// Every comment, not only the block a ';'-split schema glues above a statement. A comment between
|
|
// two keywords (`ADD /* note */ COLUMN`) is invisible to the classification regexes AND to the
|
|
// must-parse shapes, so it used to yield no target and no throw: the statement ran with no
|
|
// pre-check at all, which is the one direction this must never fail in. Postgres treats a comment
|
|
// as whitespace, so each becomes a single space. Only classification reads this; the server is
|
|
// always sent the original text.
|
|
export function sqlWithoutComments(statement: string): string {
|
|
let stripped = ''
|
|
let quote: string | undefined
|
|
for (let index = 0; index < statement.length; index += 1) {
|
|
const character = statement[index]!
|
|
if (quote !== undefined) {
|
|
stripped += character
|
|
if (character !== quote) continue
|
|
if (statement[index + 1] === quote) {
|
|
stripped += quote
|
|
index += 1
|
|
} else quote = undefined
|
|
continue
|
|
}
|
|
if (character === "'" || character === '"') {
|
|
quote = character
|
|
stripped += character
|
|
continue
|
|
}
|
|
if (character === '$') {
|
|
const end = dollarQuoteEnd(statement, index)
|
|
if (end !== undefined) {
|
|
stripped += statement.slice(index, end)
|
|
index = end - 1
|
|
continue
|
|
}
|
|
}
|
|
if (character === '-' && statement[index + 1] === '-') {
|
|
const newline = statement.indexOf('\n', index)
|
|
index = newline === -1 ? statement.length : newline
|
|
stripped += ' '
|
|
continue
|
|
}
|
|
if (character === '/' && statement[index + 1] === '*') {
|
|
// Postgres nests block comments, so a depth counter is what closes the right one.
|
|
let depth = 1
|
|
index += 2
|
|
while (index < statement.length && depth > 0) {
|
|
if (statement[index] === '/' && statement[index + 1] === '*') {
|
|
depth += 1
|
|
index += 2
|
|
} else if (statement[index] === '*' && statement[index + 1] === '/') {
|
|
depth -= 1
|
|
index += 2
|
|
} else index += 1
|
|
}
|
|
index -= 1
|
|
stripped += ' '
|
|
continue
|
|
}
|
|
stripped += character
|
|
}
|
|
return stripped.trim()
|
|
}
|
|
|
|
const CREATE_INDEX = new RegExp(
|
|
`^CREATE\\s+(?:UNIQUE\\s+)?INDEX\\s+(?:CONCURRENTLY\\s+)?(?:IF\\s+NOT\\s+EXISTS\\s+)?` +
|
|
`${QUALIFIED}\\s+ON\\s+(?:ONLY\\s+)?${QUALIFIED}`,
|
|
'i'
|
|
)
|
|
const ADD_COLUMN = new RegExp(
|
|
`^ALTER\\s+TABLE\\s+(?:IF\\s+EXISTS\\s+)?(?:ONLY\\s+)?${QUALIFIED}\\s+` +
|
|
`ADD\\s+COLUMN\\s+(?:IF\\s+NOT\\s+EXISTS\\s+)?${QUALIFIED}`,
|
|
'i'
|
|
)
|
|
const ADD_CONSTRAINT = new RegExp(
|
|
`^ALTER\\s+TABLE\\s+(?:IF\\s+EXISTS\\s+)?(?:ONLY\\s+)?${QUALIFIED}\\s+` +
|
|
`ADD\\s+CONSTRAINT\\s+${QUALIFIED}`,
|
|
'i'
|
|
)
|
|
// `IF EXISTS` is required, not optional. A bare `DROP CONSTRAINT` on a missing constraint is an
|
|
// error the server is supposed to raise, and skipping it would swallow that. Without a target the
|
|
// statement throws at boot instead, which tells the author to write `IF EXISTS`.
|
|
const DROP_CONSTRAINT = new RegExp(
|
|
`^ALTER\\s+TABLE\\s+(?:IF\\s+EXISTS\\s+)?(?:ONLY\\s+)?${QUALIFIED}\\s+` +
|
|
`DROP\\s+CONSTRAINT\\s+IF\\s+EXISTS\\s+${QUALIFIED}`,
|
|
'i'
|
|
)
|
|
|
|
// `IF EXISTS` is required for the same reason it is on DROP CONSTRAINT: a bare `DROP INDEX` on a
|
|
// missing index is an error the server is supposed to raise. Without a target the statement throws
|
|
// at boot instead, which tells the author to write `IF EXISTS`.
|
|
const DROP_INDEX = new RegExp(`^DROP\\s+INDEX\\s+(?:CONCURRENTLY\\s+)?IF\\s+EXISTS\\s+${QUALIFIED}\\s*$`, 'i')
|
|
|
|
// One option per statement, and a literal value: the catalog stores reloptions as `name=value`
|
|
// text, so the pre-check compares the written pair against that array verbatim. A list of options
|
|
// is refused by `hasTopLevelComma` before it reaches here, the same as a multi-action ALTER TABLE.
|
|
const SET_RELOPTION = new RegExp(
|
|
`^ALTER\\s+TABLE\\s+(?:IF\\s+EXISTS\\s+)?(?:ONLY\\s+)?${QUALIFIED}\\s+` +
|
|
`SET\\s+\\(\\s*([A-Za-z_][A-Za-z0-9_]*)\\s*=\\s*([A-Za-z0-9_.]+)\\s*\\)\\s*$`,
|
|
'i'
|
|
)
|
|
|
|
// Every statement shape that takes a relation lock before Postgres evaluates its existence test.
|
|
// `CREATE TABLE IF NOT EXISTS` is absent on purpose: it resolves a name against the schema and
|
|
// takes no lock on an existing table.
|
|
// `DROP INDEX` is here because it takes ACCESS EXCLUSIVE on the index's table whenever the index is
|
|
// actually there, which is every boot until the first one wins. That it takes no lock once the
|
|
// index is gone is what the pre-check turns into the steady state, not a reason to omit it.
|
|
const TAKES_RELATION_LOCK = /^(?:CREATE\s+(?:UNIQUE\s+)?INDEX|DROP\s+INDEX|ALTER\s+TABLE)\b/i
|
|
|
|
export function takesRelationLock(statement: string): boolean {
|
|
return TAKES_RELATION_LOCK.test(sqlWithoutComments(statement))
|
|
}
|
|
|
|
// Splitting on '.' is not enough: `"a.b"` is one identifier containing a dot, not two parts. Each
|
|
// part is read quote-aware, with a doubled quote unescaped to one.
|
|
function qualifiedParts(written: string): { text: string; quoted: boolean }[] {
|
|
const parts: { text: string; quoted: boolean }[] = []
|
|
let text = ''
|
|
let quoted = false
|
|
let wasQuoted = false
|
|
for (let index = 0; index < written.length; index += 1) {
|
|
const character = written[index]!
|
|
if (quoted) {
|
|
if (character !== '"') {
|
|
text += character
|
|
continue
|
|
}
|
|
if (written[index + 1] === '"') {
|
|
text += '"'
|
|
index += 1
|
|
} else quoted = false
|
|
continue
|
|
}
|
|
if (character === '"') {
|
|
quoted = true
|
|
wasQuoted = true
|
|
} else if (character === '.') {
|
|
parts.push({ text, quoted: wasQuoted })
|
|
text = ''
|
|
wasQuoted = false
|
|
} else text += character
|
|
}
|
|
parts.push({ text, quoted: wasQuoted })
|
|
return parts
|
|
}
|
|
|
|
// Postgres folds an unquoted identifier to lower case before storing it, so `Foo` is `foo` in
|
|
// relname, attname and conname. Comparing the written case would miss the row and rebuild the
|
|
// object on every boot.
|
|
function catalogName(written: string): string {
|
|
const last = qualifiedParts(written).pop()
|
|
if (!last) return written
|
|
return last.quoted ? last.text : last.text.toLowerCase()
|
|
}
|
|
|
|
// Shapes whose lock target the pre-check must be able to derive. Deliberately looser than the
|
|
// regexes that parse them, so a statement that reads as one of these but does not parse is caught
|
|
// rather than falling through to the lock path.
|
|
const MUST_PARSE = [
|
|
/^CREATE\s+(?:UNIQUE\s+)?INDEX\b/i,
|
|
/^ALTER\s+TABLE\b[\s\S]*\bADD\s+COLUMN\b/i,
|
|
/^ALTER\s+TABLE\b[\s\S]*\bADD\s+CONSTRAINT\b/i,
|
|
/^ALTER\s+TABLE\b[\s\S]*\bDROP\s+CONSTRAINT\b/i,
|
|
/^ALTER\s+TABLE\b[\s\S]*\bSET\s+\(/i,
|
|
/^DROP\s+INDEX\b/i
|
|
]
|
|
|
|
// Derived from the statement itself so a renamed index cannot drift away from its pre-check.
|
|
export function schemaLockTarget(statement: string): SchemaLockTarget | undefined {
|
|
const sql = sqlWithoutComments(statement)
|
|
const index = CREATE_INDEX.exec(sql)
|
|
if (index?.[1] && index[2]) {
|
|
return { kind: 'index', table: index[2], name: catalogName(index[1]), skipWhen: 'present' }
|
|
}
|
|
const column = ADD_COLUMN.exec(sql)
|
|
if (column?.[1] && column[2]) {
|
|
return { kind: 'column', table: column[1], name: catalogName(column[2]), skipWhen: 'present' }
|
|
}
|
|
const added = ADD_CONSTRAINT.exec(sql)
|
|
if (added?.[1] && added[2]) {
|
|
return {
|
|
kind: 'constraint',
|
|
table: added[1],
|
|
name: catalogName(added[2]),
|
|
skipWhen: 'present'
|
|
}
|
|
}
|
|
const dropped = DROP_CONSTRAINT.exec(sql)
|
|
if (dropped?.[1] && dropped[2]) {
|
|
return {
|
|
kind: 'constraint',
|
|
table: dropped[1],
|
|
name: catalogName(dropped[2]),
|
|
skipWhen: 'absent'
|
|
}
|
|
}
|
|
const droppedIndex = DROP_INDEX.exec(sql)
|
|
if (droppedIndex?.[1]) {
|
|
return { kind: 'index-by-name', name: catalogName(droppedIndex[1]), skipWhen: 'absent' }
|
|
}
|
|
const option = SET_RELOPTION.exec(sql)
|
|
if (option?.[1] && option[2] && option[3]) {
|
|
// Option names are always folded, but the value is stored as written, so only the name goes
|
|
// through catalogName. `fillfactor=70` and `fillfactor=80` are different targets, which is
|
|
// what makes a changed value re-run rather than skip.
|
|
return {
|
|
kind: 'reloption',
|
|
table: option[1],
|
|
name: `${catalogName(option[2])}=${option[3]}`,
|
|
skipWhen: 'present'
|
|
}
|
|
}
|
|
return undefined
|
|
}
|
|
|
|
const ALTER_TABLE = /^ALTER\s+TABLE\b/i
|
|
|
|
// A comma that separates ALTER TABLE subcommands rather than sitting inside a type, a default, a
|
|
// CHECK body or a dollar-quoted body. Takes comment-free SQL. Square brackets count as depth too,
|
|
// or an array type or `DEFAULT ARRAY[1, 2]` reads as a second subcommand and fails the boot.
|
|
function hasTopLevelComma(sql: string): boolean {
|
|
let depth = 0
|
|
let quote: string | undefined
|
|
for (let index = 0; index < sql.length; index += 1) {
|
|
const character = sql[index]
|
|
if (quote !== undefined) {
|
|
if (character !== quote) continue
|
|
if (sql[index + 1] === quote) index += 1
|
|
else quote = undefined
|
|
continue
|
|
}
|
|
if (character === '$') {
|
|
const end = dollarQuoteEnd(sql, index)
|
|
if (end !== undefined) {
|
|
index = end - 1
|
|
continue
|
|
}
|
|
}
|
|
if (character === "'" || character === '"') quote = character
|
|
else if (character === '(' || character === '[') depth += 1
|
|
else if (character === ')' || character === ']') depth -= 1
|
|
else if (character === ',' && depth === 0) return true
|
|
}
|
|
return false
|
|
}
|
|
|
|
// An index or column statement whose target cannot be read is the dangerous case: it would be sent
|
|
// unchecked and take the lock the pre-check exists to avoid, silently and on every boot. An
|
|
// auto-named `CREATE INDEX ON t(c)` lands here too, because nothing in the text says what the
|
|
// catalog will call it. Fail the boot with the statement instead.
|
|
export function requireSchemaLockTarget(statement: string): SchemaLockTarget | undefined {
|
|
const sql = sqlWithoutComments(statement)
|
|
// A multi-action ALTER TABLE parses to its FIRST subcommand's target only, so skipping on that
|
|
// one object would silently drop every later action for the life of the database. One action per
|
|
// statement, or no pre-check is possible.
|
|
if (ALTER_TABLE.test(sql) && hasTopLevelComma(sql)) {
|
|
throw new Error(`unparsed_schema_lock_target: ${sql}`)
|
|
}
|
|
const target = schemaLockTarget(statement)
|
|
if (target) return target
|
|
if (MUST_PARSE.some((shape) => shape.test(sql))) {
|
|
throw new Error(`unparsed_schema_lock_target: ${sql}`)
|
|
}
|
|
return undefined
|
|
}
|