fix(core): create the database when joining a racing embedded Postgres

A lifecycle that joins an already-running instance returned a connection URL
before the owner had created the database. The owner calls ensureDatabase()
only after its own start() resolves, but the signals a joiner detects the
instance by -- the runningInstances entry and, decisively, postmaster.pid,
which postgres itself writes -- both appear earlier. A joiner landing in that
window handed back a URL to a database that did not exist and failed at the
caller's first connect.

Reordering the owner's publish does not fix it: isAlreadyRunning falls back to
the pid file, whose timing postgres owns, so the joiner must verify. Both join
paths (preflight and the startup-race catch) now create the database if absent.
Creating from the joiner is safe rather than a second writer -- CREATE DATABASE
is atomic and both sides tolerate the duplicate, so whoever loses treats the
winner's database as its own success.

Verification takes the joined instance's port explicitly. getPort() resolves to
`options.port ?? resolvedPort`, which on a join with an explicitly configured
port is this instance's requested port, not the one being joined.

It is 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 logs and returns the URL exactly as before, letting the connection
layer report an unreachable cluster. A hard throw would turn every stale-pid
start into a startup failure.

Duplicate tolerance covers both codes a real cluster produces: 42P04
duplicate_database when the winner committed before our catalog probe, and
23505 unique_violation on pg_database_datname_index when the two CREATEs
collide inside the catalog insert. The concurrent-ensureDatabase test caught
the 23505 arm -- tolerating only 42P04 left the tighter half of the race
throwing.

Tests: a real-process test proving a joiner creates the database the owner has
not (drop-the-database reproduces the window), a real-process concurrent
ensureDatabase race, and an always-on test pinning the best-effort contract for
an unreachable join. All three fail without the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-15 20:46:01 -07:00
parent 71dd191c7c
commit 130c70286b
3 changed files with 225 additions and 19 deletions

View File

@@ -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`.

View File

@@ -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");

View File

@@ -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<void> {
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<void> {
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<void> {
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<boolean> {
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<boolean> {
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,