fix(webapp): reuse the primary db pool for legacy run-ops when DSNs match (#4253)
## Summary When the run-ops split is enabled, the legacy run-ops database client was always constructed as its own connection pool, even when it points at the same database as the primary (control-plane) client. On setups where those two DSNs resolve to the same physical database, this opened a second, redundant pool and doubled the number of connections used against that database. This change makes the legacy client reuse the primary client's pool whenever their DSNs point at the same database, and only open a separate pool when they genuinely differ. ## Fix A small `sameDatabaseTarget` comparison (host, port, database name, user) decides whether the legacy DSN points at the same database as the primary. When it does, the legacy handle reuses the primary client by reference, so no second pool is opened. When the DSNs diverge, the legacy client is built independently as before, so the split still works once the databases are actually separate. Two smaller changes ride along: - An optional per-pool limit for the run-ops read replica, which connects unpooled and so draws raw backend connections; unset, it falls back to the existing default and behaviour is unchanged. - A startup warning about a missing legacy replica URL is now suppressed when the legacy client shares the primary pool, where it would be misleading. ## Verification Booted the webapp end-to-end in three modes and confirmed the pools opened as expected via the client's own startup logs and live backend connection counts: split off (single pool), split on with a shared database (legacy reuses the primary pool, no doubling), and split on with separate databases (legacy opens its own pool).
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: fix
|
||||
---
|
||||
|
||||
Avoid opening a redundant database connection pool when the legacy and primary databases are the same server, preventing connection usage from doubling.
|
||||
@@ -192,6 +192,8 @@ export type SelectRunOpsTopologyConfig = {
|
||||
legacyReplicaUrl?: string;
|
||||
newUrl?: string;
|
||||
newReplicaUrl?: string;
|
||||
// When true, legacy reuses the control-plane client instead of opening its own pool. Defaults to false.
|
||||
legacySharesControlPlane?: boolean;
|
||||
};
|
||||
export type RunOpsClientBuilders = {
|
||||
controlPlane: RunOpsClients;
|
||||
@@ -226,15 +228,17 @@ export function selectRunOpsTopology(
|
||||
return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane };
|
||||
}
|
||||
|
||||
// Track 2: build an INDEPENDENT legacy client from its own DSN instead of aliasing the control
|
||||
// plane. legacyUrl is guaranteed present (the missing-URL branch above aliases and returns).
|
||||
const legacyWriter = builders.buildLegacyWriter(config.legacyUrl, "run-ops-legacy-writer");
|
||||
// Mirror the NEW replica + control-plane $replica fallback: brand a real replica (in the builder),
|
||||
// otherwise reuse the legacy WRITER so replica reads fall back to the legacy primary — unbranded.
|
||||
const legacyReplica: PrismaReplicaClient = config.legacyReplicaUrl
|
||||
? builders.buildLegacyReplica(config.legacyReplicaUrl, "run-ops-legacy-reader")
|
||||
: legacyWriter;
|
||||
const legacyRunOps: RunOpsClients = { writer: legacyWriter, replica: legacyReplica };
|
||||
// Same-DB legacy reuses the control-plane pool; only build a separate pool once the DSNs diverge.
|
||||
let legacyRunOps: RunOpsClients;
|
||||
if (config.legacySharesControlPlane) {
|
||||
legacyRunOps = controlPlane;
|
||||
} else {
|
||||
const legacyWriter = builders.buildLegacyWriter(config.legacyUrl, "run-ops-legacy-writer");
|
||||
const legacyReplica: PrismaReplicaClient = config.legacyReplicaUrl
|
||||
? builders.buildLegacyReplica(config.legacyReplicaUrl, "run-ops-legacy-reader")
|
||||
: legacyWriter;
|
||||
legacyRunOps = { writer: legacyWriter, replica: legacyReplica };
|
||||
}
|
||||
|
||||
const newWriter = builders.buildNewWriter(config.newUrl, "run-ops-new-writer");
|
||||
const newReplica: RunOpsPrismaClient = config.newReplicaUrl
|
||||
@@ -260,9 +264,19 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => {
|
||||
// Gate on the opt-in flag too: the distinct-DB sentinel only runs when the flag is on.
|
||||
const splitEnabled = env.RUN_OPS_SPLIT_ENABLED && !!newUrl && !!env.RUN_OPS_LEGACY_DATABASE_URL;
|
||||
|
||||
// Without a dedicated legacy replica URL, legacy reads fall back to the legacy WRITER (primary).
|
||||
// Surface that so a prod misdeploy is observable instead of a silent load shift onto the primary.
|
||||
if (splitEnabled && !env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL) {
|
||||
// Alias legacy onto the control-plane pool when both roles resolve to the same DB (replica URLs
|
||||
// fall back to their writer, matching how the clients themselves fall back).
|
||||
const cpWriterUrl = env.CONTROL_PLANE_DATABASE_URL ?? env.DATABASE_URL;
|
||||
const cpReplicaUrl = env.CONTROL_PLANE_DATABASE_READ_REPLICA_URL ?? env.DATABASE_READ_REPLICA_URL;
|
||||
const legacySharesControlPlane =
|
||||
sameDatabaseTarget(env.RUN_OPS_LEGACY_DATABASE_URL, cpWriterUrl) &&
|
||||
sameDatabaseTarget(
|
||||
env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL ?? env.RUN_OPS_LEGACY_DATABASE_URL,
|
||||
cpReplicaUrl ?? cpWriterUrl
|
||||
);
|
||||
|
||||
// Only meaningful for an independent legacy pool; a shared pool routes reads through $replica.
|
||||
if (splitEnabled && !legacySharesControlPlane && !env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL) {
|
||||
logger.warn(
|
||||
"RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is unset while split is enabled; legacy reads will hit the legacy primary"
|
||||
);
|
||||
@@ -275,6 +289,7 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => {
|
||||
legacyReplicaUrl: env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL,
|
||||
newUrl,
|
||||
newReplicaUrl: env.RUN_OPS_DATABASE_READ_REPLICA_URL,
|
||||
legacySharesControlPlane,
|
||||
},
|
||||
{
|
||||
controlPlane: { writer: prisma, replica: $replica },
|
||||
@@ -709,7 +724,10 @@ function buildRunOpsReplicaClient({
|
||||
clientType: string;
|
||||
}): RunOpsPrismaClient {
|
||||
const replicaUrl = extendQueryParams(url, {
|
||||
connection_limit: env.DATABASE_CONNECTION_LIMIT.toString(),
|
||||
// The new run-ops replica connects unpooled, so allow capping it independently of the writer.
|
||||
connection_limit: (
|
||||
env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT
|
||||
).toString(),
|
||||
pool_timeout: env.DATABASE_POOL_TIMEOUT.toString(),
|
||||
connection_timeout: env.DATABASE_CONNECTION_TIMEOUT.toString(),
|
||||
application_name: env.SERVICE_NAME,
|
||||
@@ -752,6 +770,25 @@ function buildRunOpsReplicaClient({
|
||||
return client;
|
||||
}
|
||||
|
||||
// True when two DSNs point at the same database (host/port/dbname/user), ignoring query params and
|
||||
// password. Parse failure or a missing URL returns false, so an unrecognized DSN just isn't aliased.
|
||||
export function sameDatabaseTarget(a: string | undefined, b: string | undefined): boolean {
|
||||
if (!a || !b) return false;
|
||||
try {
|
||||
const ua = new URL(a);
|
||||
const ub = new URL(b);
|
||||
const port = (u: URL) => u.port || "5432";
|
||||
return (
|
||||
ua.hostname.toLowerCase() === ub.hostname.toLowerCase() &&
|
||||
port(ua) === port(ub) &&
|
||||
ua.pathname === ub.pathname &&
|
||||
ua.username === ub.username
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function extendQueryParams(hrefOrUrl: string | URL, queryParams: Record<string, string>) {
|
||||
const url = new URL(hrefOrUrl);
|
||||
const query = url.searchParams;
|
||||
|
||||
@@ -159,6 +159,8 @@ const EnvironmentSchema = z
|
||||
.string()
|
||||
.refine(isValidDatabaseUrl, "RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is invalid")
|
||||
.optional(),
|
||||
// Optional cap for the unpooled new run-ops read replica. Unset falls back to DATABASE_CONNECTION_LIMIT.
|
||||
RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT: z.coerce.number().int().optional(),
|
||||
// Direct DSN for applying the full @trigger.dev/database migrations to the LEGACY run-ops DB, keeping
|
||||
// its schema current after the control plane moves off it. Direct, not pooled — migrations never run
|
||||
// over a pooler. Optional; unset -> the entrypoint's legacy migrate step is skipped.
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { PostgreSqlContainer } from "@testcontainers/postgresql";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { buildReplicaClient, buildWriterClient, selectRunOpsTopology } from "~/db.server";
|
||||
import {
|
||||
buildReplicaClient,
|
||||
buildWriterClient,
|
||||
sameDatabaseTarget,
|
||||
selectRunOpsTopology,
|
||||
} from "~/db.server";
|
||||
|
||||
const cp = { writer: {} as any, replica: {} as any };
|
||||
|
||||
@@ -44,7 +49,34 @@ describe("selectRunOpsTopology (pure)", () => {
|
||||
expect(buildLegacyWriter).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("split ON: legacy builds its OWN writer + replica (Track 2: no longer aliased to control-plane)", () => {
|
||||
it("split ON + legacySharesControlPlane: aliases legacy to control-plane and builds NO legacy client", () => {
|
||||
const newWriter = { tag: "nw" } as any;
|
||||
const newReplica = { tag: "nr" } as any;
|
||||
const buildNewWriter = vi.fn().mockReturnValue(newWriter);
|
||||
const buildNewReplica = vi.fn().mockReturnValue(newReplica);
|
||||
const buildLegacyWriter = vi.fn();
|
||||
const buildLegacyReplica = vi.fn();
|
||||
const topo = selectRunOpsTopology(
|
||||
{
|
||||
splitEnabled: true,
|
||||
legacyUrl: "postgres://same",
|
||||
legacyReplicaUrl: "postgres://same-r",
|
||||
newUrl: "postgres://new",
|
||||
newReplicaUrl: "postgres://new-r",
|
||||
legacySharesControlPlane: true,
|
||||
},
|
||||
{ controlPlane: cp, buildNewWriter, buildNewReplica, buildLegacyWriter, buildLegacyReplica }
|
||||
);
|
||||
// Legacy reuses the control-plane pair by reference — no second pool against the same server.
|
||||
expect(topo.legacyRunOps).toBe(cp);
|
||||
expect(buildLegacyWriter).not.toHaveBeenCalled();
|
||||
expect(buildLegacyReplica).not.toHaveBeenCalled();
|
||||
// New run-ops still builds its own (independent) client.
|
||||
expect(topo.newRunOps.writer).toBe(newWriter);
|
||||
expect(topo.newRunOps.replica).toBe(newReplica);
|
||||
});
|
||||
|
||||
it("split ON (flag off): legacy builds its OWN writer + replica (independent, not aliased)", () => {
|
||||
const newWriter = { tag: "nw" } as any;
|
||||
const newReplica = { tag: "nr" } as any;
|
||||
const legacyWriter = { tag: "lw" } as any;
|
||||
@@ -112,6 +144,49 @@ describe("selectRunOpsTopology (pure)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("sameDatabaseTarget", () => {
|
||||
it("same host/port/db/user is a match despite differing query params and password", () => {
|
||||
expect(
|
||||
sameDatabaseTarget(
|
||||
"postgresql://user:secret1@db.internal:5432/trigger?connection_limit=10&application_name=api",
|
||||
"postgresql://user:secret2@db.internal:5432/trigger?connection_limit=55"
|
||||
)
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats a missing port as the default 5432", () => {
|
||||
expect(
|
||||
sameDatabaseTarget(
|
||||
"postgresql://user@db.internal/trigger",
|
||||
"postgresql://user@db.internal:5432/trigger"
|
||||
)
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("differs on host, port, dbname, or user", () => {
|
||||
const base = "postgresql://user@db.internal:5432/trigger";
|
||||
expect(sameDatabaseTarget(base, "postgresql://user@other.internal:5432/trigger")).toBe(false);
|
||||
expect(sameDatabaseTarget(base, "postgresql://user@db.internal:6432/trigger")).toBe(false);
|
||||
expect(sameDatabaseTarget(base, "postgresql://user@db.internal:5432/other")).toBe(false);
|
||||
expect(sameDatabaseTarget(base, "postgresql://other@db.internal:5432/trigger")).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for undefined or unparseable input", () => {
|
||||
expect(sameDatabaseTarget(undefined, "postgresql://user@db/trigger")).toBe(false);
|
||||
expect(sameDatabaseTarget("postgresql://user@db/trigger", undefined)).toBe(false);
|
||||
expect(sameDatabaseTarget("not a url", "also not a url")).toBe(false);
|
||||
});
|
||||
|
||||
it("matches the prod-shaped legacy-vs-cp writer pair, not the new replica", () => {
|
||||
const cpWriter = "postgresql://master:pw@rds-writer.internal:5432/pgtrigger";
|
||||
const legacyWriter =
|
||||
"postgresql://master:pw@rds-writer.internal:5432/pgtrigger?connection_limit=25";
|
||||
const newReplica = "postgresql://master%7Creplica:pw@ps-host.internal:5432/pgtrigger";
|
||||
expect(sameDatabaseTarget(cpWriter, legacyWriter)).toBe(true);
|
||||
expect(sameDatabaseTarget(cpWriter, newReplica)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("selectRunOpsTopology (integration, real containers)", () => {
|
||||
it("split OFF: opens exactly one DB; all run-ops handles share the control-plane client", async () => {
|
||||
const pg = await new PostgreSqlContainer("docker.io/postgres:14").start();
|
||||
@@ -152,7 +227,7 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it("split ON: constructs CP + INDEPENDENT legacy-run-ops + new-run-ops + replicas", async () => {
|
||||
it("split ON (flag off): constructs CP + INDEPENDENT legacy-run-ops + new-run-ops + replicas", async () => {
|
||||
const rds = await new PostgreSqlContainer("docker.io/postgres:14").start();
|
||||
const ps = await new PostgreSqlContainer("docker.io/postgres:17").start();
|
||||
try {
|
||||
@@ -161,8 +236,8 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
|
||||
const topo = selectRunOpsTopology(
|
||||
{
|
||||
splitEnabled: true,
|
||||
// Same-DSN stage: legacy points at the same physical DB as the control plane, but the
|
||||
// client is an INDEPENDENT instance (its own pool) — never the cp object.
|
||||
// Divergent-DB stage (legacySharesControlPlane omitted): legacy builds an INDEPENDENT
|
||||
// client with its own pool — never the cp object.
|
||||
legacyUrl: rds.getConnectionUri(),
|
||||
legacyReplicaUrl: rds.getConnectionUri(),
|
||||
newUrl: ps.getConnectionUri(),
|
||||
@@ -195,4 +270,47 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
|
||||
await ps.stop();
|
||||
}
|
||||
}, 120_000);
|
||||
|
||||
it("split ON + legacySharesControlPlane: legacy reuses the CP pool, only the new DB opens a client", async () => {
|
||||
const rds = await new PostgreSqlContainer("docker.io/postgres:14").start();
|
||||
const ps = await new PostgreSqlContainer("docker.io/postgres:17").start();
|
||||
try {
|
||||
const cpWriter = buildWriterClient({ url: rds.getConnectionUri(), clientType: "cp" });
|
||||
const cp = { writer: cpWriter, replica: cpWriter };
|
||||
const legacyBuilds: string[] = [];
|
||||
const topo = selectRunOpsTopology(
|
||||
{
|
||||
splitEnabled: true,
|
||||
legacyUrl: rds.getConnectionUri(),
|
||||
legacyReplicaUrl: rds.getConnectionUri(),
|
||||
newUrl: ps.getConnectionUri(),
|
||||
legacySharesControlPlane: true,
|
||||
},
|
||||
{
|
||||
controlPlane: cp,
|
||||
buildNewWriter: (url, ct) => buildWriterClient({ url, clientType: ct }) as any,
|
||||
buildNewReplica: (url, ct) => buildReplicaClient({ url, clientType: ct }) as any,
|
||||
buildLegacyWriter: (url, ct) => {
|
||||
legacyBuilds.push(url);
|
||||
return buildWriterClient({ url, clientType: ct });
|
||||
},
|
||||
buildLegacyReplica: (url, ct) => {
|
||||
legacyBuilds.push(url);
|
||||
return buildReplicaClient({ url, clientType: ct });
|
||||
},
|
||||
}
|
||||
);
|
||||
expect(legacyBuilds).toHaveLength(0); // no redundant legacy pool against the shared server
|
||||
expect(topo.legacyRunOps).toBe(cp);
|
||||
expect(topo.legacyRunOps.writer).toBe(cpWriter);
|
||||
expect(topo.newRunOps.writer).not.toBe(cpWriter);
|
||||
await topo.legacyRunOps.writer.$queryRawUnsafe("SELECT 1"); // legacy queries run on the CP pool
|
||||
await topo.newRunOps.writer.$queryRawUnsafe("SELECT 1");
|
||||
await cpWriter.$disconnect();
|
||||
await topo.newRunOps.writer.$disconnect();
|
||||
} finally {
|
||||
await rds.stop();
|
||||
await ps.stop();
|
||||
}
|
||||
}, 120_000);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user