diff --git a/.changeset/embedded-postgres-join-database-race.md b/.changeset/embedded-postgres-join-database-race.md new file mode 100644 index 0000000000..b244b44866 --- /dev/null +++ b/.changeset/embedded-postgres-join-database-race.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix a startup failure when two Fusion processes start embedded Postgres at the same time. +category: fix +dev: A lifecycle joining an already-running instance returned a URL before the owner's `ensureDatabase()` had created the database, so the joiner's first connect failed. Both join paths now verify the database on the joined instance's port (never `getPort()`, which prefers this instance's requested port) and create it if absent. Verification is best-effort so an unreachable/stale-pid join still resolves optimistically as before. `CREATE DATABASE` races tolerate both `42P04` and `23505` on `pg_database_datname_index`. diff --git a/packages/core/src/__tests__/postgres/embedded-lifecycle.test.ts b/packages/core/src/__tests__/postgres/embedded-lifecycle.test.ts index ae0731605e..47b57b0b85 100644 --- a/packages/core/src/__tests__/postgres/embedded-lifecycle.test.ts +++ b/packages/core/src/__tests__/postgres/embedded-lifecycle.test.ts @@ -628,6 +628,96 @@ embeddedDescribe("embedded-lifecycle: real process (VAL-CONN-001, VAL-CONN-006, REAL_PROCESS_TEST_TIMEOUT_MS, ); + /* + FNXC:PostgresStartupRace 2026-07-15-20:45: + The owner creates the database only after its own start() resolves, but a joiner detects the + instance earlier (registry entry, then postmaster.pid — written by postgres itself). A joiner + landing in that window used to hand back a URL to a database that did not exist yet. Dropping + the database from a started cluster reproduces exactly that state: postmaster up and + published, database absent. + */ + it( + "a joining lifecycle creates the database when the owner has not yet", + async () => { + const dataDir = makeDataDir(); + const owner = new EmbeddedPostgresLifecycle(baseOptions(dataDir)); + tracked.push({ lifecycle: owner, dataDir }); + await owner.start(); + const port = owner.getPort()!; + + const openAdmin = () => + postgres({ + host: "localhost", + port, + user: "postgres", + password: "password", + database: "postgres", + max: 1, + connect_timeout: 10, + }); + + // Rewind to the race window: cluster running, database not yet created. + const admin = openAdmin(); + try { + await admin.unsafe(`DROP DATABASE IF EXISTS "fusion"`); + } finally { + await admin.end({ timeout: 5 }); + } + + const joiner = new EmbeddedPostgresLifecycle(baseOptions(dataDir)); + const resolved = await joiner.start(); + + // It joined rather than starting its own postmaster... + expect(joiner.isRunning()).toBe(false); + expect(resolved.runtimeUrl).toContain(`:${port}/`); + + // ...and did not hand back a URL to a database that does not exist. + const check = openAdmin(); + try { + const rows = await check`SELECT 1 AS one FROM pg_database WHERE datname = 'fusion'`; + expect(rows.length).toBe(1); + } finally { + await check.end({ timeout: 5 }); + } + }, + REAL_PROCESS_TEST_TIMEOUT_MS, + ); + + /* FNXC:PostgresStartupRace 2026-07-15-20:45: both the owner's ensureDatabase and the join + path create the database, so a concurrent CREATE must resolve as success (42P04), not throw. + Racing them against one cluster is the only honest way to exercise that tolerance. */ + it( + "concurrent ensureDatabase calls tolerate a duplicate-database race", + async () => { + const dataDir = makeDataDir(); + const lifecycle = new EmbeddedPostgresLifecycle(baseOptions(dataDir)); + tracked.push({ lifecycle, dataDir }); + await lifecycle.start(); + const port = lifecycle.getPort()!; + + const admin = postgres({ + host: "localhost", + port, + user: "postgres", + password: "password", + database: "postgres", + max: 1, + connect_timeout: 10, + }); + try { + await admin.unsafe(`DROP DATABASE IF EXISTS "fusion"`); + } finally { + await admin.end({ timeout: 5 }); + } + + // Both see "missing" and both issue CREATE DATABASE; one must lose and survive it. + await expect( + Promise.all([lifecycle.ensureDatabase(), lifecycle.ensureDatabase()]), + ).resolves.toBeDefined(); + }, + REAL_PROCESS_TEST_TIMEOUT_MS, + ); + it( "graceful shutdown stops the Postgres process; no orphan remains (VAL-CONN-007)", async () => { @@ -737,6 +827,46 @@ describe("embedded-lifecycle: startup race (cross-process)", () => { }); }); +/* +FNXC:PostgresStartupRace 2026-07-15-20:45: +Mocked ctor, no real Postgres — kept outside the real-process block so it runs under the +gate/CI default (see the sibling startup-race block for why that placement matters). + +Pins the best-effort half of the join-path database verify: `isAlreadyRunning` joins +optimistically without probing (a stale pid file from a crash still resolves to a port), so an +unreachable joined instance must return the URL exactly as it did before the verify existed and +let the connection layer report it. A hard throw here would turn every stale-pid start into a +startup failure. +*/ +describe("embedded-lifecycle: join-path database verify is best-effort", () => { + it("still resolves optimistically when the joined instance is unreachable", async () => { + const dataDir = makeDataDir(); + writeFileSync(join(dataDir, "PG_VERSION"), "15\n"); + // A port nothing is listening on: the verify's probe cannot succeed. + writeFileSync( + join(dataDir, "postmaster.pid"), + ["12345", dataDir, "/tmp", "localhost", "55441", "5432101", String(Date.now())].join("\n") + "\n", + ); + const logLines: string[] = []; + + try { + const lifecycle = new EmbeddedPostgresLifecycle({ + ...baseOptions(dataDir), + onLog: (message) => logLines.push(message), + }); + + await expect(lifecycle.start()).resolves.toMatchObject({ + mode: "embedded", + runtimeUrl: expect.stringContaining(":55441/"), + }); + expect(lifecycle.isRunning()).toBe(false); + expect(logLines.some((line) => /could not verify database/i.test(line))).toBe(true); + } finally { + rmSync(dataDir, { recursive: true, force: true }); + } + }); +}); + describe("embedded-lifecycle: startup timeout (P1 #24)", () => { it("EmbeddedStartTimeoutError carries the timeout and data dir", () => { const err = new EmbeddedStartTimeoutError(5000, "/tmp/data"); diff --git a/packages/core/src/postgres/embedded-lifecycle.ts b/packages/core/src/postgres/embedded-lifecycle.ts index 06ef135fd2..659eface9d 100644 --- a/packages/core/src/postgres/embedded-lifecycle.ts +++ b/packages/core/src/postgres/embedded-lifecycle.ts @@ -779,6 +779,24 @@ export function readPortFromPostmasterPid(dataDir: string): number | null { * Uses both the in-process registry AND a probe of the postmaster.pid file * (handles the case where another process started it). */ +/** + * True when a failed `CREATE DATABASE` means someone else already created it. + * + * FNXC:PostgresStartupRace 2026-07-15-20:45: + * `42P04` duplicate_database is the documented code, raised when the winner committed before we + * probed the catalog. A tighter collision — both statements inside the `pg_database` insert — + * instead surfaces `23505` unique_violation on `pg_database_datname_index`. Scope the 23505 arm + * to that constraint so an unrelated unique violation still throws. + */ +function isDuplicateDatabaseError(error: unknown): boolean { + const { code, constraint_name: constraint } = (error ?? {}) as { + code?: string; + constraint_name?: string; + }; + if (code === "42P04") return true; + return code === "23505" && constraint === "pg_database_datname_index"; +} + function isAlreadyRunning(dataDir: string): { port: number; database: string } | null { // Check in-process registry first const cached = runningInstances.get(dataDir); @@ -984,12 +1002,11 @@ export class EmbeddedPostgresLifecycle { this.running = false; // We didn't start it, so we won't stop it this.ownsProcess = false; - /* - FNXC:PostgresCutover 2026-07-15-20:14: - This path deliberately does NOT create the database, despite what this comment claimed for the preceding months — there has never been an `ensureDatabase()` call here. The process that owns the postmaster creates it after its own start(); a joiner has no cluster of its own to ensure. `ensureDatabase()` would throw here regardless: it requires `this.running`, which the join path leaves false by design so `stop()` never reaps an instance we did not start. - - Assumption worth knowing: the owner publishes `runningInstances` / writes `postmaster.pid` BEFORE its `ensureDatabase()` resolves, so a joiner that wins that narrow window connects to a not-yet-created database and fails at the connection layer rather than here. Callers see a connect error, not a silent empty DB. - */ + // FNXC:PostgresStartupRace 2026-07-15-20:45: the owner may not have created the + // database yet — it does so only after its own start() resolves, while the signals + // that brought us here appear earlier. Verify against the joined instance's port + // (never getPort(), which prefers our own requested port). See ensureJoinedDatabase. + await this.ensureJoinedDatabase(existing.port); const url = this.buildUrl(existing.port, this.options.database); return { mode: "embedded", @@ -1142,6 +1159,10 @@ export class EmbeddedPostgresLifecycle { this.options.onLog( `embedded postgres: startup raced with an existing instance on port ${existing.port} (data dir ${this.options.dataDir}), connecting without starting a new instance`, ); + // FNXC:PostgresStartupRace 2026-07-15-20:45: this is the tightest window of all — we + // lost the race by milliseconds, so the winner's ensureDatabase() is very likely still + // in flight. Same best-effort verify as the preflight join. + await this.ensureJoinedDatabase(existing.port); const runtimeUrl = this.buildUrl(existing.port, this.options.database); return { mode: "embedded", @@ -1239,26 +1260,78 @@ export class EmbeddedPostgresLifecycle { * SQL connection with a bounded connect timeout instead. */ async ensureDatabase(): Promise { - if (!this.running || this.getPort() === undefined) { + const port = this.getPort(); + if (!this.running || port === undefined) { throw new Error( "Cannot ensure database: the embedded cluster is not running. Call start() first.", ); } - const exists = await this.databaseExists(this.options.database); - if (exists) return; - const sql = this.openMaintenanceSql(); + await this.createDatabaseIfMissing(port); + } + + /** + * Join path: make sure the database exists on an instance THIS process does not own. + * + * FNXC:PostgresStartupRace 2026-07-15-20:45: + * The owner creates the database only after its own start() resolves, but the signals a + * joiner detects it by — the `runningInstances` entry and, decisively, `postmaster.pid` + * (written by postgres itself) — both appear BEFORE that. A joiner winning the window + * therefore handed back a URL to a database that did not exist yet and failed at the + * caller's first connect. Reordering the owner's publish cannot fix it: `isAlreadyRunning` + * falls back to the pid file, whose timing postgres owns, so the joiner must verify. + * + * Creating it here is safe rather than a second writer: `CREATE DATABASE` is atomic, and + * both this path and the owner's `ensureDatabase` tolerate `42P04`, so whoever loses the + * race treats the winner's database as its own success. + * + * Best-effort by contract. `isAlreadyRunning` joins optimistically without probing (a stale + * pid file from a crash still resolves to a port), so a probe failure must leave that + * behavior exactly as it was — report it and return the URL, letting the connection layer + * surface an unreachable cluster as it always has. Never convert an optimistic join into a + * hard startup failure. + */ + private async ensureJoinedDatabase(port: number): Promise { + try { + await this.createDatabaseIfMissing(port); + } catch (error) { + this.options.onLog( + `embedded postgres: could not verify database "${this.options.database}" on joined instance at port ${port} (${error instanceof Error ? error.message : String(error)}); continuing — the connection layer will report an unreachable cluster`, + ); + } + } + + /** + * Create `options.database` on the cluster at `port` unless it already exists. + * + * Takes an explicit port because {@link getPort} resolves to `options.port ?? resolvedPort` + * — on a join with an explicitly configured port that is THIS instance's requested port, + * not the port of the instance actually being joined. + */ + private async createDatabaseIfMissing(port: number): Promise { + if (await this.databaseExistsOn(port, this.options.database)) return; + const sql = this.openMaintenanceSqlOn(port); try { const safeName = this.options.database.replace(/"/g, '""'); await sql.unsafe(`CREATE DATABASE "${safeName}"`); + } catch (error) { + // FNXC:PostgresStartupRace 2026-07-15-20:45: a concurrent starter or joiner created the + // database between our existence check and this statement. The post-condition we promise + // (the database exists) holds, so that is success, not an error. + // + // Two distinct codes, both observed against a real cluster: 42P04 duplicate_database when + // the winner committed before we checked the catalog, and 23505 unique_violation on + // pg_database_datname_index when the two CREATEs collide inside the catalog insert itself. + // Tolerating only 42P04 leaves the tighter half of the race throwing — which is exactly + // what the concurrent-ensureDatabase test caught. + if (!isDuplicateDatabaseError(error)) throw error; } finally { await sql.end({ timeout: 5 }).catch(() => {}); } } - /** Check whether a database with the given name exists on the cluster. */ - private async databaseExists(name: string): Promise { - if (!this.running || this.getPort() === undefined) return false; - const sql = this.openMaintenanceSql(); + /** Check whether a database with the given name exists on the cluster at `port`. */ + private async databaseExistsOn(port: number, name: string): Promise { + const sql = this.openMaintenanceSqlOn(port); try { const rows = await sql`SELECT 1 AS one FROM pg_database WHERE datname = ${name}`; return rows.length > 0; @@ -1279,11 +1352,7 @@ export class EmbeddedPostgresLifecycle { * library start() — unavailable on the elevated Windows non-admin path. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - private openMaintenanceSql(): any { - const port = this.getPort(); - if (port === undefined) { - throw new Error("openMaintenanceSql: no port assigned"); - } + private openMaintenanceSqlOn(port: number): any { const host = process.platform === "win32" ? "127.0.0.1" : "localhost"; return postgres({ host,