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..97a3ae7bb53 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -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) { 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..e81cd056321 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -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. 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); });