mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 16:02:15 +00:00
Jinwoo Hong
8e8a9b38ea
* 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.
68 lines
3.5 KiB
TypeScript
68 lines
3.5 KiB
TypeScript
import type { SchemaLockTarget } from './schema-lock-target.js'
|
|
|
|
export type SchemaCatalogRow = Record<string, unknown>
|
|
|
|
// Runs with `$n` placeholders bound to the lock target, on the same connection the DDL would use.
|
|
export type SchemaCatalogQuery = (
|
|
sql: string,
|
|
params: unknown[]
|
|
) => Promise<SchemaCatalogRow[]>
|
|
|
|
// The index name is resolved inside the table's own namespace, and `i.indrelid = t.oid` ties it to
|
|
// this table: index names are unique per schema, not per table, so without that condition a
|
|
// same-named index on a sibling table answers yes and the real index is skipped forever.
|
|
// `to_regclass` returns NULL rather than erroring when the table does not exist yet, which is the
|
|
// whole of a cold start.
|
|
const INDEX_PRESENT = `SELECT i.indisvalid FROM pg_catalog.pg_class t
|
|
JOIN pg_catalog.pg_class c ON c.relnamespace = t.relnamespace AND c.relname = $2
|
|
JOIN pg_catalog.pg_index i ON i.indexrelid = c.oid AND i.indrelid = t.oid
|
|
WHERE t.oid = to_regclass($1)`
|
|
|
|
const COLUMN_PRESENT = `SELECT 1 FROM pg_catalog.pg_attribute
|
|
WHERE attrelid = to_regclass($1) AND attname = $2 AND attnum > 0 AND NOT attisdropped`
|
|
|
|
// Name only. The CHECK body is generated from RELAY_REGIONS, so comparing it would re-run the swap
|
|
// on every region change, and an ADD CONSTRAINT is the one statement here that scans the table.
|
|
const CONSTRAINT_PRESENT = `SELECT 1 FROM pg_catalog.pg_constraint
|
|
WHERE conrelid = to_regclass($1) AND conname = $2`
|
|
|
|
// By name through the search_path, with no table condition, because a DROP INDEX has no table to
|
|
// condition on and does not need one: a name that resolves to no visible index is nothing to drop.
|
|
// `relkind = 'i'` keeps a same-named table or view from answering for an index. Partitioned indexes
|
|
// are 'I', which this deliberately does not match - relay has none, and dropping one is not a
|
|
// boot-time operation.
|
|
const INDEX_BY_NAME_PRESENT = `SELECT 1 FROM pg_catalog.pg_class c
|
|
WHERE c.relname = $1 AND c.relkind = 'i' AND pg_catalog.pg_table_is_visible(c.oid)`
|
|
|
|
// reloptions is a text[] of `name=value` pairs, absent entirely while the option is at its
|
|
// default. Comparing the whole pair is what makes a changed value re-run: `@>` on a different
|
|
// value answers no, and the statement runs and overwrites it.
|
|
const RELOPTION_PRESENT = `SELECT 1 FROM pg_catalog.pg_class
|
|
WHERE oid = to_regclass($1) AND reloptions @> ARRAY[$2]`
|
|
|
|
const PRESENCE_SQL = {
|
|
index: INDEX_PRESENT,
|
|
column: COLUMN_PRESENT,
|
|
constraint: CONSTRAINT_PRESENT,
|
|
reloption: RELOPTION_PRESENT,
|
|
'index-by-name': INDEX_BY_NAME_PRESENT
|
|
} as const
|
|
|
|
export type SchemaCatalogPresence = { present: boolean; indisvalid: unknown }
|
|
|
|
// Row presence is the answer, whatever the row says. An index left invalid by a cancelled
|
|
// concurrent build is skipped by `IF NOT EXISTS` today as well, so reading `indisvalid` as a
|
|
// condition would newly take the lock for exactly the indexes a failed build left behind.
|
|
export async function catalogObjectPresence(
|
|
query: SchemaCatalogQuery,
|
|
target: SchemaLockTarget
|
|
): Promise<SchemaCatalogPresence> {
|
|
const sql = PRESENCE_SQL[target.kind]
|
|
// The name-only lookup binds one parameter; every other shape binds the table first. Passing a
|
|
// parameter the SQL never references is a bind error, not a harmless extra.
|
|
const params = target.kind === 'index-by-name' ? [target.name] : [target.table, target.name]
|
|
const rows = await query(sql, params)
|
|
const row = rows[0]
|
|
return row ? { present: true, indisvalid: row.indisvalid } : { present: false, indisvalid: undefined }
|
|
}
|