Skip to content

Commit 0bbf20c

Browse files
committed
fix(webapp): reuse the primary db pool for legacy run-ops when DSNs match
When the run-ops split is enabled, the legacy run-ops Prisma client was always built as its own connection pool, even when it targeted the same physical database as the primary (control-plane) client. Where those DSNs resolve to the same database, that opened a second, redundant pool and doubled the connections used against that database. Reuse the primary client by reference when the legacy and primary DSNs resolve to the same database (host, port, database name, user), and only build a separate legacy pool when they genuinely differ.
1 parent 1659557 commit 0bbf20c

4 files changed

Lines changed: 194 additions & 18 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Avoid opening a redundant database connection pool when the legacy and primary databases are the same server, preventing connection usage from doubling.

apps/webapp/app/db.server.ts

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,11 @@ export type SelectRunOpsTopologyConfig = {
192192
legacyReplicaUrl?: string;
193193
newUrl?: string;
194194
newReplicaUrl?: string;
195+
// When the legacy DSN targets the same physical DB as the control plane (the pre-cutover reality),
196+
// reuse the control-plane client by reference instead of opening a second, redundant pool against
197+
// the same server. The env-bound singleton computes this via sameDatabaseTarget(). Defaults to false
198+
// (build an independent legacy client — the post-cutover / distinct-DB behaviour).
199+
legacySharesControlPlane?: boolean;
195200
};
196201
export type RunOpsClientBuilders = {
197202
controlPlane: RunOpsClients;
@@ -226,15 +231,20 @@ export function selectRunOpsTopology(
226231
return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane };
227232
}
228233

229-
// Track 2: build an INDEPENDENT legacy client from its own DSN instead of aliasing the control
230-
// plane. legacyUrl is guaranteed present (the missing-URL branch above aliases and returns).
231-
const legacyWriter = builders.buildLegacyWriter(config.legacyUrl, "run-ops-legacy-writer");
232-
// Mirror the NEW replica + control-plane $replica fallback: brand a real replica (in the builder),
233-
// otherwise reuse the legacy WRITER so replica reads fall back to the legacy primary — unbranded.
234-
const legacyReplica: PrismaReplicaClient = config.legacyReplicaUrl
235-
? builders.buildLegacyReplica(config.legacyReplicaUrl, "run-ops-legacy-reader")
236-
: legacyWriter;
237-
const legacyRunOps: RunOpsClients = { writer: legacyWriter, replica: legacyReplica };
234+
// When the legacy DSN targets the same physical DB as the control plane (true pre-cutover), reuse
235+
// the control-plane client by reference — no second pool against the same server, so split-on can't
236+
// double RDS connections. buildLegacy* runs only once the DSNs diverge (post control-plane cutover).
237+
let legacyRunOps: RunOpsClients;
238+
if (config.legacySharesControlPlane) {
239+
legacyRunOps = controlPlane;
240+
} else {
241+
const legacyWriter = builders.buildLegacyWriter(config.legacyUrl, "run-ops-legacy-writer");
242+
// Brand a real replica (in the builder), otherwise fall back to the legacy WRITER — unbranded.
243+
const legacyReplica: PrismaReplicaClient = config.legacyReplicaUrl
244+
? builders.buildLegacyReplica(config.legacyReplicaUrl, "run-ops-legacy-reader")
245+
: legacyWriter;
246+
legacyRunOps = { writer: legacyWriter, replica: legacyReplica };
247+
}
238248

239249
const newWriter = builders.buildNewWriter(config.newUrl, "run-ops-new-writer");
240250
const newReplica: RunOpsPrismaClient = config.newReplicaUrl
@@ -260,9 +270,20 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => {
260270
// Gate on the opt-in flag too: the distinct-DB sentinel only runs when the flag is on.
261271
const splitEnabled = env.RUN_OPS_SPLIT_ENABLED && !!newUrl && !!env.RUN_OPS_LEGACY_DATABASE_URL;
262272

263-
// Without a dedicated legacy replica URL, legacy reads fall back to the legacy WRITER (primary).
264-
// Surface that so a prod misdeploy is observable instead of a silent load shift onto the primary.
265-
if (splitEnabled && !env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL) {
273+
// Reuse the control-plane pool for legacy when both roles target the same physical DB (compare the
274+
// effective URLs, applying the same writer fallback each replica uses when its own URL is unset).
275+
const cpWriterUrl = env.CONTROL_PLANE_DATABASE_URL ?? env.DATABASE_URL;
276+
const cpReplicaUrl = env.CONTROL_PLANE_DATABASE_READ_REPLICA_URL ?? env.DATABASE_READ_REPLICA_URL;
277+
const legacySharesControlPlane =
278+
sameDatabaseTarget(env.RUN_OPS_LEGACY_DATABASE_URL, cpWriterUrl) &&
279+
sameDatabaseTarget(
280+
env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL ?? env.RUN_OPS_LEGACY_DATABASE_URL,
281+
cpReplicaUrl ?? cpWriterUrl
282+
);
283+
284+
// Only meaningful for an INDEPENDENT legacy pool: without its own replica URL, legacy reads fall
285+
// back to the legacy primary. A shared pool routes through $replica instead, so the warning is moot.
286+
if (splitEnabled && !legacySharesControlPlane && !env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL) {
266287
logger.warn(
267288
"RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is unset while split is enabled; legacy reads will hit the legacy primary"
268289
);
@@ -275,6 +296,7 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => {
275296
legacyReplicaUrl: env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL,
276297
newUrl,
277298
newReplicaUrl: env.RUN_OPS_DATABASE_READ_REPLICA_URL,
299+
legacySharesControlPlane,
278300
},
279301
{
280302
controlPlane: { writer: prisma, replica: $replica },
@@ -709,7 +731,11 @@ function buildRunOpsReplicaClient({
709731
clientType: string;
710732
}): RunOpsPrismaClient {
711733
const replicaUrl = extendQueryParams(url, {
712-
connection_limit: env.DATABASE_CONNECTION_LIMIT.toString(),
734+
// The new run-ops replica is UNPOOLED (direct to the replica host), so it draws raw backend
735+
// connections. Cap it independently when set; otherwise behave exactly as before.
736+
connection_limit: (
737+
env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT
738+
).toString(),
713739
pool_timeout: env.DATABASE_POOL_TIMEOUT.toString(),
714740
connection_timeout: env.DATABASE_CONNECTION_TIMEOUT.toString(),
715741
application_name: env.SERVICE_NAME,
@@ -752,6 +778,27 @@ function buildRunOpsReplicaClient({
752778
return client;
753779
}
754780

781+
// True when two DSNs point at the same physical database (host/port/dbname/user), ignoring query
782+
// params (connection_limit etc. differ) and password. Parse failure -> false: don't alias, just build
783+
// independently (no correctness risk, only a missed dedup). Used to decide whether the legacy run-ops
784+
// client can share the control-plane pool instead of opening a redundant one against the same server.
785+
export function sameDatabaseTarget(a: string | undefined, b: string | undefined): boolean {
786+
if (!a || !b) return false;
787+
try {
788+
const ua = new URL(a);
789+
const ub = new URL(b);
790+
const port = (u: URL) => u.port || "5432";
791+
return (
792+
ua.hostname.toLowerCase() === ub.hostname.toLowerCase() &&
793+
port(ua) === port(ub) &&
794+
ua.pathname === ub.pathname &&
795+
ua.username === ub.username
796+
);
797+
} catch {
798+
return false;
799+
}
800+
}
801+
755802
function extendQueryParams(hrefOrUrl: string | URL, queryParams: Record<string, string>) {
756803
const url = new URL(hrefOrUrl);
757804
const query = url.searchParams;

apps/webapp/app/env.server.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,11 @@ const EnvironmentSchema = z
159159
.string()
160160
.refine(isValidDatabaseUrl, "RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is invalid")
161161
.optional(),
162+
// Optional per-pool cap for the NEW run-ops read-replica client only. The new replica connects
163+
// direct (unpooled) to the run-ops replica host, so unlike the pooled writer it draws raw backend
164+
// connections against that host's max_connections. Unset -> falls back to DATABASE_CONNECTION_LIMIT
165+
// (behaviour identical to today), letting ops throttle just the unpooled replica without a redeploy.
166+
RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT: z.coerce.number().int().optional(),
162167
// Direct DSN for applying the full @trigger.dev/database migrations to the LEGACY run-ops DB, keeping
163168
// its schema current after the control plane moves off it. Direct, not pooled — migrations never run
164169
// over a pooler. Optional; unset -> the entrypoint's legacy migrate step is skipped.

apps/webapp/test/runOpsDbTopology.test.ts

Lines changed: 123 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { PostgreSqlContainer } from "@testcontainers/postgresql";
22
import { describe, expect, it, vi } from "vitest";
3-
import { buildReplicaClient, buildWriterClient, selectRunOpsTopology } from "~/db.server";
3+
import {
4+
buildReplicaClient,
5+
buildWriterClient,
6+
sameDatabaseTarget,
7+
selectRunOpsTopology,
8+
} from "~/db.server";
49

510
const cp = { writer: {} as any, replica: {} as any };
611

@@ -44,7 +49,34 @@ describe("selectRunOpsTopology (pure)", () => {
4449
expect(buildLegacyWriter).not.toHaveBeenCalled();
4550
});
4651

47-
it("split ON: legacy builds its OWN writer + replica (Track 2: no longer aliased to control-plane)", () => {
52+
it("split ON + legacySharesControlPlane: aliases legacy to control-plane and builds NO legacy client", () => {
53+
const newWriter = { tag: "nw" } as any;
54+
const newReplica = { tag: "nr" } as any;
55+
const buildNewWriter = vi.fn().mockReturnValue(newWriter);
56+
const buildNewReplica = vi.fn().mockReturnValue(newReplica);
57+
const buildLegacyWriter = vi.fn();
58+
const buildLegacyReplica = vi.fn();
59+
const topo = selectRunOpsTopology(
60+
{
61+
splitEnabled: true,
62+
legacyUrl: "postgres://same",
63+
legacyReplicaUrl: "postgres://same-r",
64+
newUrl: "postgres://new",
65+
newReplicaUrl: "postgres://new-r",
66+
legacySharesControlPlane: true,
67+
},
68+
{ controlPlane: cp, buildNewWriter, buildNewReplica, buildLegacyWriter, buildLegacyReplica }
69+
);
70+
// Legacy reuses the control-plane pair by reference — no second pool against the same server.
71+
expect(topo.legacyRunOps).toBe(cp);
72+
expect(buildLegacyWriter).not.toHaveBeenCalled();
73+
expect(buildLegacyReplica).not.toHaveBeenCalled();
74+
// New run-ops still builds its own (independent) client.
75+
expect(topo.newRunOps.writer).toBe(newWriter);
76+
expect(topo.newRunOps.replica).toBe(newReplica);
77+
});
78+
79+
it("split ON (flag off): legacy builds its OWN writer + replica (independent, not aliased)", () => {
4880
const newWriter = { tag: "nw" } as any;
4981
const newReplica = { tag: "nr" } as any;
5082
const legacyWriter = { tag: "lw" } as any;
@@ -112,6 +144,49 @@ describe("selectRunOpsTopology (pure)", () => {
112144
});
113145
});
114146

147+
describe("sameDatabaseTarget", () => {
148+
it("same host/port/db/user is a match despite differing query params and password", () => {
149+
expect(
150+
sameDatabaseTarget(
151+
"postgresql://user:secret1@db.internal:5432/trigger?connection_limit=10&application_name=api",
152+
"postgresql://user:secret2@db.internal:5432/trigger?connection_limit=55"
153+
)
154+
).toBe(true);
155+
});
156+
157+
it("treats a missing port as the default 5432", () => {
158+
expect(
159+
sameDatabaseTarget(
160+
"postgresql://user@db.internal/trigger",
161+
"postgresql://user@db.internal:5432/trigger"
162+
)
163+
).toBe(true);
164+
});
165+
166+
it("differs on host, port, dbname, or user", () => {
167+
const base = "postgresql://user@db.internal:5432/trigger";
168+
expect(sameDatabaseTarget(base, "postgresql://user@other.internal:5432/trigger")).toBe(false);
169+
expect(sameDatabaseTarget(base, "postgresql://user@db.internal:6432/trigger")).toBe(false);
170+
expect(sameDatabaseTarget(base, "postgresql://user@db.internal:5432/other")).toBe(false);
171+
expect(sameDatabaseTarget(base, "postgresql://other@db.internal:5432/trigger")).toBe(false);
172+
});
173+
174+
it("returns false for undefined or unparseable input", () => {
175+
expect(sameDatabaseTarget(undefined, "postgresql://user@db/trigger")).toBe(false);
176+
expect(sameDatabaseTarget("postgresql://user@db/trigger", undefined)).toBe(false);
177+
expect(sameDatabaseTarget("not a url", "also not a url")).toBe(false);
178+
});
179+
180+
it("matches the prod-shaped legacy-vs-cp writer pair, not the new replica", () => {
181+
const cpWriter = "postgresql://master:pw@rds-writer.internal:5432/pgtrigger";
182+
const legacyWriter =
183+
"postgresql://master:pw@rds-writer.internal:5432/pgtrigger?connection_limit=25";
184+
const newReplica = "postgresql://master%7Creplica:pw@ps-host.internal:5432/pgtrigger";
185+
expect(sameDatabaseTarget(cpWriter, legacyWriter)).toBe(true);
186+
expect(sameDatabaseTarget(cpWriter, newReplica)).toBe(false);
187+
});
188+
});
189+
115190
describe("selectRunOpsTopology (integration, real containers)", () => {
116191
it("split OFF: opens exactly one DB; all run-ops handles share the control-plane client", async () => {
117192
const pg = await new PostgreSqlContainer("docker.io/postgres:14").start();
@@ -152,7 +227,7 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
152227
}
153228
}, 60_000);
154229

155-
it("split ON: constructs CP + INDEPENDENT legacy-run-ops + new-run-ops + replicas", async () => {
230+
it("split ON (flag off): constructs CP + INDEPENDENT legacy-run-ops + new-run-ops + replicas", async () => {
156231
const rds = await new PostgreSqlContainer("docker.io/postgres:14").start();
157232
const ps = await new PostgreSqlContainer("docker.io/postgres:17").start();
158233
try {
@@ -161,8 +236,8 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
161236
const topo = selectRunOpsTopology(
162237
{
163238
splitEnabled: true,
164-
// Same-DSN stage: legacy points at the same physical DB as the control plane, but the
165-
// client is an INDEPENDENT instance (its own pool) — never the cp object.
239+
// Divergent-DB stage (legacySharesControlPlane omitted): legacy builds an INDEPENDENT
240+
// client with its own pool — never the cp object.
166241
legacyUrl: rds.getConnectionUri(),
167242
legacyReplicaUrl: rds.getConnectionUri(),
168243
newUrl: ps.getConnectionUri(),
@@ -195,4 +270,47 @@ describe("selectRunOpsTopology (integration, real containers)", () => {
195270
await ps.stop();
196271
}
197272
}, 120_000);
273+
274+
it("split ON + legacySharesControlPlane: legacy reuses the CP pool, only the new DB opens a client", async () => {
275+
const rds = await new PostgreSqlContainer("docker.io/postgres:14").start();
276+
const ps = await new PostgreSqlContainer("docker.io/postgres:17").start();
277+
try {
278+
const cpWriter = buildWriterClient({ url: rds.getConnectionUri(), clientType: "cp" });
279+
const cp = { writer: cpWriter, replica: cpWriter };
280+
const legacyBuilds: string[] = [];
281+
const topo = selectRunOpsTopology(
282+
{
283+
splitEnabled: true,
284+
legacyUrl: rds.getConnectionUri(),
285+
legacyReplicaUrl: rds.getConnectionUri(),
286+
newUrl: ps.getConnectionUri(),
287+
legacySharesControlPlane: true,
288+
},
289+
{
290+
controlPlane: cp,
291+
buildNewWriter: (url, ct) => buildWriterClient({ url, clientType: ct }) as any,
292+
buildNewReplica: (url, ct) => buildReplicaClient({ url, clientType: ct }) as any,
293+
buildLegacyWriter: (url, ct) => {
294+
legacyBuilds.push(url);
295+
return buildWriterClient({ url, clientType: ct });
296+
},
297+
buildLegacyReplica: (url, ct) => {
298+
legacyBuilds.push(url);
299+
return buildReplicaClient({ url, clientType: ct });
300+
},
301+
}
302+
);
303+
expect(legacyBuilds).toHaveLength(0); // no redundant legacy pool against the shared server
304+
expect(topo.legacyRunOps).toBe(cp);
305+
expect(topo.legacyRunOps.writer).toBe(cpWriter);
306+
expect(topo.newRunOps.writer).not.toBe(cpWriter);
307+
await topo.legacyRunOps.writer.$queryRawUnsafe("SELECT 1"); // legacy queries run on the CP pool
308+
await topo.newRunOps.writer.$queryRawUnsafe("SELECT 1");
309+
await cpWriter.$disconnect();
310+
await topo.newRunOps.writer.$disconnect();
311+
} finally {
312+
await rds.stop();
313+
await ps.stop();
314+
}
315+
}, 120_000);
198316
});

0 commit comments

Comments
 (0)