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:
7
.changeset/embedded-postgres-join-database-race.md
Normal file
7
.changeset/embedded-postgres-join-database-race.md
Normal 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`.
|
||||
@@ -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");
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user