From 0bbf20c67f403a7a37f7e7d71dc622e2260a36ca Mon Sep 17 00:00:00 2001 From: Dan Sutton Date: Tue, 14 Jul 2026 11:03:36 +0100 Subject: [PATCH 1/2] 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. --- .server-changes/run-ops-legacy-shared-pool.md | 6 + apps/webapp/app/db.server.ts | 73 ++++++++-- apps/webapp/app/env.server.ts | 5 + apps/webapp/test/runOpsDbTopology.test.ts | 128 +++++++++++++++++- 4 files changed, 194 insertions(+), 18 deletions(-) create mode 100644 .server-changes/run-ops-legacy-shared-pool.md diff --git a/.server-changes/run-ops-legacy-shared-pool.md b/.server-changes/run-ops-legacy-shared-pool.md new file mode 100644 index 00000000000..dca8d5192b9 --- /dev/null +++ b/.server-changes/run-ops-legacy-shared-pool.md @@ -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. diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 9fb251fa452..86b85e2f176 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -192,6 +192,11 @@ export type SelectRunOpsTopologyConfig = { legacyReplicaUrl?: string; newUrl?: string; newReplicaUrl?: string; + // When the legacy DSN targets the same physical DB as the control plane (the pre-cutover reality), + // reuse the control-plane client by reference instead of opening a second, redundant pool against + // the same server. The env-bound singleton computes this via sameDatabaseTarget(). Defaults to false + // (build an independent legacy client — the post-cutover / distinct-DB behaviour). + legacySharesControlPlane?: boolean; }; export type RunOpsClientBuilders = { controlPlane: RunOpsClients; @@ -226,15 +231,20 @@ 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 }; + // When the legacy DSN targets the same physical DB as the control plane (true pre-cutover), reuse + // the control-plane client by reference — no second pool against the same server, so split-on can't + // double RDS connections. buildLegacy* runs only once the DSNs diverge (post control-plane cutover). + let legacyRunOps: RunOpsClients; + if (config.legacySharesControlPlane) { + legacyRunOps = controlPlane; + } else { + const legacyWriter = builders.buildLegacyWriter(config.legacyUrl, "run-ops-legacy-writer"); + // Brand a real replica (in the builder), otherwise fall back to the legacy WRITER — unbranded. + 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 +270,20 @@ 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) { + // Reuse the control-plane pool for legacy when both roles target the same physical DB (compare the + // effective URLs, applying the same writer fallback each replica uses when its own URL is unset). + 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: without its own replica URL, legacy reads fall + // back to the legacy primary. A shared pool routes through $replica instead, so the warning is moot. + 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 +296,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 +731,11 @@ function buildRunOpsReplicaClient({ clientType: string; }): RunOpsPrismaClient { const replicaUrl = extendQueryParams(url, { - connection_limit: env.DATABASE_CONNECTION_LIMIT.toString(), + // The new run-ops replica is UNPOOLED (direct to the replica host), so it draws raw backend + // connections. Cap it independently when set; otherwise behave exactly as before. + 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 +778,27 @@ function buildRunOpsReplicaClient({ return client; } +// True when two DSNs point at the same physical database (host/port/dbname/user), ignoring query +// params (connection_limit etc. differ) and password. Parse failure -> false: don't alias, just build +// independently (no correctness risk, only a missed dedup). Used to decide whether the legacy run-ops +// client can share the control-plane pool instead of opening a redundant one against the same server. +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) { const url = new URL(hrefOrUrl); const query = url.searchParams; diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 97414fb2368..3205079bc64 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -159,6 +159,11 @@ const EnvironmentSchema = z .string() .refine(isValidDatabaseUrl, "RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is invalid") .optional(), + // Optional per-pool cap for the NEW run-ops read-replica client only. The new replica connects + // direct (unpooled) to the run-ops replica host, so unlike the pooled writer it draws raw backend + // connections against that host's max_connections. Unset -> falls back to DATABASE_CONNECTION_LIMIT + // (behaviour identical to today), letting ops throttle just the unpooled replica without a redeploy. + 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. diff --git a/apps/webapp/test/runOpsDbTopology.test.ts b/apps/webapp/test/runOpsDbTopology.test.ts index 1e69ad0a4dc..8890fdbb662 100644 --- a/apps/webapp/test/runOpsDbTopology.test.ts +++ b/apps/webapp/test/runOpsDbTopology.test.ts @@ -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); }); From 6c4eed06b2b8724171fb00896ba4c48f2b326480 Mon Sep 17 00:00:00 2001 From: Dan Sutton Date: Tue, 14 Jul 2026 11:06:36 +0100 Subject: [PATCH 2/2] chore(webapp): trim run-ops pool comments --- apps/webapp/app/db.server.ts | 26 ++++++++------------------ apps/webapp/app/env.server.ts | 5 +---- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 86b85e2f176..97a3ae7bb53 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -192,10 +192,7 @@ export type SelectRunOpsTopologyConfig = { legacyReplicaUrl?: string; newUrl?: string; newReplicaUrl?: string; - // When the legacy DSN targets the same physical DB as the control plane (the pre-cutover reality), - // reuse the control-plane client by reference instead of opening a second, redundant pool against - // the same server. The env-bound singleton computes this via sameDatabaseTarget(). Defaults to false - // (build an independent legacy client — the post-cutover / distinct-DB behaviour). + // When true, legacy reuses the control-plane client instead of opening its own pool. Defaults to false. legacySharesControlPlane?: boolean; }; export type RunOpsClientBuilders = { @@ -231,15 +228,12 @@ export function selectRunOpsTopology( return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane }; } - // When the legacy DSN targets the same physical DB as the control plane (true pre-cutover), reuse - // the control-plane client by reference — no second pool against the same server, so split-on can't - // double RDS connections. buildLegacy* runs only once the DSNs diverge (post control-plane cutover). + // 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"); - // Brand a real replica (in the builder), otherwise fall back to the legacy WRITER — unbranded. const legacyReplica: PrismaReplicaClient = config.legacyReplicaUrl ? builders.buildLegacyReplica(config.legacyReplicaUrl, "run-ops-legacy-reader") : legacyWriter; @@ -270,8 +264,8 @@ 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; - // Reuse the control-plane pool for legacy when both roles target the same physical DB (compare the - // effective URLs, applying the same writer fallback each replica uses when its own URL is unset). + // 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 = @@ -281,8 +275,7 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { cpReplicaUrl ?? cpWriterUrl ); - // Only meaningful for an INDEPENDENT legacy pool: without its own replica URL, legacy reads fall - // back to the legacy primary. A shared pool routes through $replica instead, so the warning is moot. + // 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" @@ -731,8 +724,7 @@ function buildRunOpsReplicaClient({ clientType: string; }): RunOpsPrismaClient { const replicaUrl = extendQueryParams(url, { - // The new run-ops replica is UNPOOLED (direct to the replica host), so it draws raw backend - // connections. Cap it independently when set; otherwise behave exactly as before. + // 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(), @@ -778,10 +770,8 @@ function buildRunOpsReplicaClient({ return client; } -// True when two DSNs point at the same physical database (host/port/dbname/user), ignoring query -// params (connection_limit etc. differ) and password. Parse failure -> false: don't alias, just build -// independently (no correctness risk, only a missed dedup). Used to decide whether the legacy run-ops -// client can share the control-plane pool instead of opening a redundant one against the same server. +// 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 { diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 3205079bc64..e81cd056321 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -159,10 +159,7 @@ const EnvironmentSchema = z .string() .refine(isValidDatabaseUrl, "RUN_OPS_LEGACY_DATABASE_READ_REPLICA_URL is invalid") .optional(), - // Optional per-pool cap for the NEW run-ops read-replica client only. The new replica connects - // direct (unpooled) to the run-ops replica host, so unlike the pooled writer it draws raw backend - // connections against that host's max_connections. Unset -> falls back to DATABASE_CONNECTION_LIMIT - // (behaviour identical to today), letting ops throttle just the unpooled replica without a redeploy. + // 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