|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// #14176 — the primary datasource's knex pool had no operator-facing size. |
| 4 | +// |
| 5 | +// What the reporter measured (a live 3-replica EE cluster, Postgres 16 with |
| 6 | +// `max_connections=200`, 2026-09-01) and what this file does NOT re-measure: |
| 7 | +// authed data throughput plateaued at ~25 rps while Postgres held ~9-21 of its |
| 8 | +// 200 connections, i.e. the ceiling was the client pool, not the server. Those |
| 9 | +// are the REPORTER'S numbers. No test here reproduces them — there is no |
| 10 | +// cluster and no live database in this suite, and a fabricated local rerun |
| 11 | +// would be evidence of nothing. What is pinnable here is the MECHANISM the |
| 12 | +// throughput number rests on: which pool size actually reaches knex. |
| 13 | +// |
| 14 | +// The card blamed `SqlDriver.withConnectBound` for setting no pool size. That |
| 15 | +// is falsified for this path and the pins below say why: the CLI composes the |
| 16 | +// primary datasource as `config: { url, ...autoMigrate }` with no `pool` block |
| 17 | +// (`packages/cli/src/utils/storage-driver.ts`, the `postgres` arm), so |
| 18 | +// `buildSqlPool` — not knex's own `{min:2,max:10}` default, and not the driver |
| 19 | +// — decides the size, and it decided `{min:0,max:5}` for every deployment. |
| 20 | +// An env read in the driver would have been dead code behind that explicit |
| 21 | +// object. Maintainer ruling 2026-09-02 (「同意」, option A): read one variable, |
| 22 | +// `OS_DATABASE_POOL_MAX`, in `buildSqlPool`, precedence declared `pool` > env > |
| 23 | +// `{min:0,max:5}`. |
| 24 | +// |
| 25 | +// ⚠️ The first pin below is the load-bearing one. An unset env is the upgrade |
| 26 | +// path for every existing deployment, so "unset means exactly what it meant |
| 27 | +// before" is the whole safety argument for this change; if it ever goes red, |
| 28 | +// the change is silently altering production connection counts on upgrade. |
| 29 | + |
| 30 | +import { describe, it, expect, afterEach } from 'vitest'; |
| 31 | +import { createDefaultDatasourceDriverFactory } from '../default-datasource-driver-factory.js'; |
| 32 | + |
| 33 | +const factory = () => createDefaultDatasourceDriverFactory({ dev: false }); |
| 34 | + |
| 35 | +const POOL_MAX_ENV = 'OS_DATABASE_POOL_MAX'; |
| 36 | + |
| 37 | +/** The knex config a constructed SqlDriver was built from (as the sibling suite reads it). */ |
| 38 | +function knexConfigOf(driver: any): any { |
| 39 | + return driver?.config ?? driver?.knexConfig ?? driver?.options ?? {}; |
| 40 | +} |
| 41 | + |
| 42 | +/** |
| 43 | + * Build a datasource the way the CLI composes the PRIMARY one — a url and |
| 44 | + * nothing else, in particular no `pool` block — and return its knex pool. |
| 45 | + * Never connects: `create` constructs the driver, and the pool is read off the |
| 46 | + * config it was built from. |
| 47 | + */ |
| 48 | +async function primaryPool( |
| 49 | + extra: Record<string, unknown> = {}, |
| 50 | +): Promise<Record<string, unknown>> { |
| 51 | + const handle: any = await factory().create({ |
| 52 | + driver: 'postgres', |
| 53 | + config: { url: 'postgres://app@db.internal:5432/app' }, |
| 54 | + ...extra, |
| 55 | + }); |
| 56 | + try { |
| 57 | + return knexConfigOf(handle.driver ?? handle).pool; |
| 58 | + } finally { |
| 59 | + try { await handle.disconnect?.(); } catch { /* pool never opened */ } |
| 60 | + } |
| 61 | +} |
| 62 | + |
| 63 | +function setEnv(value: string | undefined): void { |
| 64 | + if (value === undefined) delete process.env[POOL_MAX_ENV]; |
| 65 | + else process.env[POOL_MAX_ENV] = value; |
| 66 | +} |
| 67 | + |
| 68 | +afterEach(() => { delete process.env[POOL_MAX_ENV]; }); |
| 69 | + |
| 70 | +describe('OS_DATABASE_POOL_MAX — the primary datasource pool ceiling (#14176)', () => { |
| 71 | + // ⭐ THE DEFAULT-PRESERVATION PIN. Do not relax this one. |
| 72 | + it('with the env UNSET, the pool is byte-identical to the pre-change default', async () => { |
| 73 | + setEnv(undefined); |
| 74 | + expect(await primaryPool()).toEqual({ min: 0, max: 5 }); |
| 75 | + }); |
| 76 | + |
| 77 | + it('reads a blank value as unset rather than as garbage', async () => { |
| 78 | + // `OS_DATABASE_POOL_MAX=` in a compose file is a declared-but-unfilled |
| 79 | + // variable asking for today's behaviour, not a misconfiguration. |
| 80 | + setEnv(''); |
| 81 | + expect(await primaryPool()).toEqual({ min: 0, max: 5 }); |
| 82 | + setEnv(' '); |
| 83 | + expect(await primaryPool()).toEqual({ min: 0, max: 5 }); |
| 84 | + }); |
| 85 | + |
| 86 | + it('raises the ceiling the operator asked for, leaving the floor alone', async () => { |
| 87 | + setEnv('40'); |
| 88 | + // The floor stays 0 — `OS_DATABASE_POOL_MIN` is deliberately NOT exposed |
| 89 | + // (ruling 2026-09-02: today's path runs `min: 0`; a later patch if needed). |
| 90 | + expect(await primaryPool()).toEqual({ min: 0, max: 40 }); |
| 91 | + }); |
| 92 | + |
| 93 | + it("a datasource's own declared pool outranks the operator env", async () => { |
| 94 | + setEnv('40'); |
| 95 | + const pool = await primaryPool({ pool: { min: 2, max: 9 } }); |
| 96 | + expect(pool).toMatchObject({ min: 2, max: 9 }); |
| 97 | + }); |
| 98 | + |
| 99 | + it('refuses a non-integer value loudly instead of silently keeping the default', async () => { |
| 100 | + // The failure this rejects is the lenient `Number(process.env.X ?? d)` |
| 101 | + // shape: a typo becomes NaN, knex gets an unsizable pool, and the operator |
| 102 | + // who was TRYING to raise the ceiling silently keeps the one they wanted |
| 103 | + // to leave. A pool ceiling is only ever measured in production. |
| 104 | + for (const bad of ['abc', '10.5', '-4', '0', '1e3', '0x10', '10 20']) { |
| 105 | + setEnv(bad); |
| 106 | + await expect( |
| 107 | + factory().create({ driver: 'postgres', config: { url: 'postgres://app@db.internal:5432/app' } }), |
| 108 | + ).rejects.toThrow(/OS_DATABASE_POOL_MAX must be a positive integer/); |
| 109 | + } |
| 110 | + }); |
| 111 | + |
| 112 | + it('names the variable, the value it rejected and the sizing rule in the refusal', async () => { |
| 113 | + setEnv('lots'); |
| 114 | + await expect( |
| 115 | + factory().create({ driver: 'postgres', config: { url: 'postgres://app@db.internal:5432/app' } }), |
| 116 | + ).rejects.toThrow(/got "lots"[\s\S]*max_connections/); |
| 117 | + }); |
| 118 | + |
| 119 | + it('applies to the mysql arm on the same terms', async () => { |
| 120 | + setEnv('12'); |
| 121 | + const handle: any = await factory().create({ |
| 122 | + driver: 'mysql', |
| 123 | + config: { url: 'mysql://app@db.internal:3306/app' }, |
| 124 | + }); |
| 125 | + try { |
| 126 | + expect(knexConfigOf(handle.driver ?? handle).pool).toEqual({ min: 0, max: 12 }); |
| 127 | + } finally { |
| 128 | + try { await handle.disconnect?.(); } catch { /* pool never opened */ } |
| 129 | + } |
| 130 | + }); |
| 131 | + |
| 132 | + // ⛔ The guard for the constraint this change must not breach. `sqlite` / |
| 133 | + // `sqlite-wasm` / `memory` / `turso` REJECT a declared `pool` outright under |
| 134 | + // three maintainer rulings (#5714 / #5931 / #7243), and knex's better-sqlite3 |
| 135 | + // dialect pins `{min:1,max:1}` on purpose — two connections to `:memory:` are |
| 136 | + // two separate databases. The env is read inside `buildSqlPool`, which only |
| 137 | + // the `postgres` / `mysql` arms call, so those arms structurally cannot see |
| 138 | + // it. This pin is what makes "structurally" a measurement. |
| 139 | + it('does not leak a pool onto an arm that refuses to be pooled', async () => { |
| 140 | + setEnv('40'); |
| 141 | + const handle: any = await factory().create({ driver: 'sqlite', config: { filename: ':memory:' } }); |
| 142 | + try { |
| 143 | + const cfg = knexConfigOf(handle.driver ?? handle); |
| 144 | + expect(cfg.pool?.max).not.toBe(40); |
| 145 | + } finally { |
| 146 | + try { await handle.disconnect?.(); } catch { /* pool never opened */ } |
| 147 | + } |
| 148 | + }); |
| 149 | +}); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments