Skip to content

Commit 32d3800

Browse files
authored
fix(driver-sql): bound a connection attempt at 10s, correct the "no reconnection" claim (#3769) (#3781)
Two related corrections, both from measuring what #3741/#3751/#3765 had only asserted. **The claim was wrong.** #3751 and #3765 shipped several statements that drivers never reconnect. Measured, both recover: killing a real mongod and restarting it on the same port, the SAME driver instance served the next write in 13ms with no reconnect call from us (the official driver's topology monitor); and a knex/pg pool is not poisoned by an outage — its error tracks live server state, i.e. every acquire opens a fresh connection. The original reasoning grepped this repo for `reconnect`, found nothing, and concluded recovery does not happen — but the recovery lives in the client libraries, not in our code. **Fail-fast at boot is unchanged; the reason is different.** It is not that the connection can never return — it is that the boot sequence never re-runs. A driver that missed init() also missed syncRegisteredSchemas(), so its tables can simply not exist even after the database comes back. The DEGRADED BOOT banner now says that. **The real defect underneath.** SqlDriver passed its config to knex untouched, so an endpoint that accepts TCP but never completes the handshake made every query wait out tarn's 30s default, then fail with "Timeout acquiring a connection. The pool is probably full" — pointing an operator at pool sizing instead of the network. SqlDriver now defaults pool.createTimeoutMillis to 10s, matching driver-mongodb's existing connectTimeoutMS. A host that sets its own value is left alone. Message accuracy needs the dialect-specific knob (pg's connectionTimeoutMillis), which changes the shape of `connection` and would regress serve.ts's startup banner — left open in #3769.
1 parent aff9e56 commit 32d3800

9 files changed

Lines changed: 216 additions & 32 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/objectql": patch
4+
"@objectstack/types": patch
5+
---
6+
7+
fix(driver-sql): bound a connection attempt at 10s, and correct the "no reconnection" claim (#3769, #3759)
8+
9+
Two related corrections, both from measuring what #3741/#3751/#3765 had only asserted.
10+
11+
**The claim was wrong.** #3751 and #3765 shipped several statements that drivers
12+
never reconnect — "there is no lazy reconnection", "NOT retried and NOT
13+
reconnected", "stays disconnected for the process lifetime". Measured, both
14+
drivers recover on their own:
15+
16+
- driver-mongodb: killing a real `mongod` and restarting it on the same port,
17+
the *same* driver instance served the next write successfully (13ms), with no
18+
reconnect call from us — the official driver's topology monitor handles it.
19+
- driver-sql: a knex/pg pool is not poisoned by an outage. Its error tracks live
20+
server state (`ECONNREFUSED` while down → a handshake error once a listener is
21+
back → `ECONNREFUSED` again), i.e. every acquire opens a fresh connection.
22+
`storage-driver.ts` also configures `pool.min: 0`, so no stale idle
23+
connections are held.
24+
25+
The original reasoning grepped this repo for `reconnect`, found nothing, and
26+
concluded recovery does not happen — but the recovery lives in the client
27+
libraries, not in our code. The claims are now corrected in `DriverConnectError`,
28+
the `DEGRADED BOOT` banner, `resolveAllowDriverConnectFailure`'s docs, and the
29+
drivers / self-hosting pages.
30+
31+
**Fail-fast at boot is unchanged and still correct** — the reason is just
32+
different. It is not that the connection can never return; it is that the *boot
33+
sequence* never re-runs. A driver that missed `init()` also missed
34+
`syncRegisteredSchemas()`, so its tables can simply not exist even after the
35+
database comes back. The banner now says that.
36+
37+
**The real defect underneath.** `SqlDriver` passed its config to knex untouched,
38+
so a database endpoint that accepts TCP but never completes the handshake — an
39+
overloaded instance, a half-open firewall, a load balancer mid-failover — made
40+
every query wait out tarn's 30s default, then fail with `Timeout acquiring a
41+
connection. The pool is probably full`, pointing an operator at pool sizing
42+
instead of the network. With a small `pool.max` a few such queries saturate the
43+
pool and everything else queues.
44+
45+
`SqlDriver` now defaults `pool.createTimeoutMillis` to **10s**, matching
46+
driver-mongodb's existing `connectTimeoutMS ?? 10_000` so both drivers give up on
47+
an unreachable server at the same point. A host that sets its own
48+
`createTimeoutMillis` is left alone.
49+
50+
**Migration.** None for a healthy datasource. A deployment that deliberately
51+
relies on connection establishment taking longer than 10s (a slow cross-region
52+
replica) should set `pool.createTimeoutMillis` explicitly on its `SqlDriver`
53+
config.
54+
55+
Not fixed here, tracked in #3769: knex still reports the bounded wait as "the
56+
pool is probably full". An accurate message needs a dialect-specific connect
57+
timeout (pg's `connectionTimeoutMillis`), which changes the shape of `connection`
58+
and would regress the startup banner's URL display.

content/docs/data-modeling/drivers.mdx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,24 @@ bootstrap. If any `connect()` rejects, the boot is **refused**
7878
([#3741](https://github.com/objectstack-ai/objectstack/issues/3741)) — the error
7979
names each failed driver and its cause, and `objectstack serve` exits 1.
8080

81-
There is no lazy reconnection. A driver that did not connect at startup stays
82-
disconnected for the process lifetime, so booting anyway would produce a server
83-
that reports itself started, answers health checks, and then fails every request
84-
with an error nothing like *the database is unreachable* — while the schema sync
85-
that follows `init()` issues DDL against a datasource that isn't there.
81+
Booting without a reachable datasource would produce a server that reports
82+
itself started, answers health checks, and then fails every request with an
83+
error nothing like *the database is unreachable* — while the schema sync that
84+
follows `init()` issues DDL against a datasource that isn't there.
85+
86+
The reason is **not** that the connection can never come back. Client libraries
87+
do re-establish connections on their own — the MongoDB driver's topology monitor
88+
reconnects, and knex/pg opens a fresh connection per acquire (verified in
89+
[#3759](https://github.com/objectstack-ai/objectstack/issues/3759)). What never
90+
comes back is the **boot sequence that was skipped**: nothing re-runs the schema
91+
sync, so those objects can be left with no tables even after the database
92+
returns.
8693

8794
<Callout type="warn">
8895
`OS_ALLOW_DRIVER_CONNECT_FAILURE=1` boots anyway, in an explicitly degraded
89-
state announced by a `DEGRADED BOOT` banner. The failed drivers are never
90-
retried and never reconnected: every query and every schema sync routed to them
91-
fails until the process restarts. Do not set it in production.
96+
state announced by a `DEGRADED BOOT` banner. Queries to a failed driver fail
97+
until the datasource becomes reachable, and its boot-time schema sync is skipped
98+
for good. Do not set it in production.
9299
</Callout>
93100

94101
### Writing a driver: `connect()` is where you refuse to start

content/docs/deployment/self-hosting.mdx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,14 @@ cleanly and a replica that lost its database stops receiving traffic instead of
298298
serving 500s. Before setting `replicas > 1`, read the multi-node note below.
299299

300300
<Callout type="info">
301-
Drivers do **not** reconnect on their own
302-
([#3759](https://github.com/objectstack-ai/objectstack/issues/3759)). Once a
303-
connection is lost the process stays broken until it restarts, so on Kubernetes
304-
the readiness probe above is what removes the replica — pair it with your
305-
normal restart policy. Without an orchestrator (bare `os serve`, systemd, a
306-
single container with no health check), plan for a supervisor that restarts on
307-
a failing `/api/v1/ready`.
301+
A replica **does** recover on its own once the database returns — client
302+
libraries re-establish connections without help (the MongoDB driver's topology
303+
monitor; knex/pg opening a fresh connection per acquire, verified in
304+
[#3759](https://github.com/objectstack-ai/objectstack/issues/3759)). So a
305+
transient outage — a failover, a maintenance window — needs no restart: the
306+
readiness probe drains the replica while the database is away and re-admits it
307+
afterwards. Restart only when the process is genuinely stuck, and note that a
308+
replica which *booted* without its database never re-runs its schema sync.
308309
</Callout>
309310

310311
## Reverse proxy & TLS

packages/objectql/src/driver-connect-errors.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ export class DriverConnectError extends Error {
6767
super(
6868
`${failures.length} of ${totalDrivers} data driver(s) failed to connect — refusing to boot.\n` +
6969
`${detail}\n` +
70-
`A driver that did not connect cannot serve queries (there is no lazy reconnection) and the ` +
71-
`schema sync that runs right after init would issue DDL against it. Fix the datasource ` +
72-
`configuration (e.g. OS_DATABASE_URL), or set OS_ALLOW_DRIVER_CONNECT_FAILURE=1 to boot anyway ` +
73-
`in an explicitly degraded state where every query to those drivers fails.`,
70+
`A driver that did not connect cannot serve queries, and the schema sync that runs right ` +
71+
`after init would issue DDL against it. Fix the datasource configuration (e.g. ` +
72+
`OS_DATABASE_URL), or set OS_ALLOW_DRIVER_CONNECT_FAILURE=1 to boot anyway and serve errors ` +
73+
`until the datasource becomes reachable.`,
7474
);
7575
this.name = 'DriverConnectError';
7676
if (failures[0]?.error instanceof Error) {

packages/objectql/src/engine-driver-connect-failfast.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ describe('ObjectQL.init() — driver connect fail-fast (framework#3741)', () =>
132132
await expect(engine.init()).resolves.toBeUndefined();
133133
const warned = warnings.join('\n');
134134
expect(warned).toContain('DEGRADED BOOT');
135-
expect(warned).toContain('NOT reconnected');
135+
// The durable consequence is the SKIPPED SCHEMA SYNC, not a dead connection:
136+
// the client reconnects on its own, but nothing re-runs the DDL (#3759).
137+
expect(warned).toContain('SKIPPED FOR GOOD');
136138
expect(warned).toContain('sql');
137139
});
138140

packages/objectql/src/engine.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,13 +1891,15 @@ export class ObjectQL implements IDataEngine {
18911891
* `connect()` rejects, this throws {@link DriverConnectError} and kernel
18921892
* bootstrap aborts. Two reasons, both load-bearing:
18931893
*
1894-
* 1. A driver that did not connect never recovers — there is no lazy
1895-
* reconnection anywhere in `driver-sql` / `driver-mongodb`. Booting
1896-
* anyway produces a server that reports itself started, may even answer
1897-
* health checks, and then 500s every request with an error that reads
1894+
* 1. Booting without a reachable datasource produces a server that reports
1895+
* itself started and then 500s every request with an error that reads
18981896
* nothing like "the database is unreachable". The caller immediately
18991897
* makes it worse: `ObjectQLPlugin.start()` runs `syncRegisteredSchemas()`
1900-
* right after this, issuing DDL against a driver that isn't there.
1898+
* right after this, issuing DDL against a driver that isn't there — so
1899+
* the boot leaves the schema half-applied even if the database appears
1900+
* a second later. Recovery is not the point: the underlying clients DO
1901+
* re-establish connections on their own (framework#3759 verified this),
1902+
* but nothing re-runs the boot sequence that was skipped.
19011903
* 2. Swallowing the rejection removed a driver's ability to REFUSE STARTUP.
19021904
* Any fatal startup check — licence, server version, incompatible
19031905
* configuration, missing capability, not just an unreachable socket — is
@@ -1935,9 +1937,10 @@ export class ObjectQL implements IDataEngine {
19351937
const banner =
19361938
`⚠️ DEGRADED BOOT: ${failures.length} of ${this.drivers.size} driver(s) failed to connect ` +
19371939
`(${failedDrivers.join(', ')}), but OS_ALLOW_DRIVER_CONNECT_FAILURE is set — starting anyway. ` +
1938-
`These drivers are NOT retried and NOT reconnected: every query and every schema sync routed ` +
1939-
`to them fails for the lifetime of this process. Unset OS_ALLOW_DRIVER_CONNECT_FAILURE to ` +
1940-
`restore fail-fast boot.`;
1940+
`Every query routed to them fails until the datasource becomes reachable, and the boot-time ` +
1941+
`schema sync is SKIPPED FOR GOOD — the client will reconnect on its own, but nothing re-runs ` +
1942+
`the DDL, so those objects may have no tables even after the database comes back. Unset ` +
1943+
`OS_ALLOW_DRIVER_CONNECT_FAILURE to restore fail-fast boot.`;
19411944
this.logger.warn(banner, { failedDrivers });
19421945
// …and again on a channel the host cannot silence — see the helper's note
19431946
// on `os serve`'s boot-quiet stdout capture.
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// framework#3769: a database endpoint that accepts the TCP connection but never
4+
// completes the handshake (overloaded instance, half-open firewall, LB mid-
5+
// failover) makes every query WAIT rather than fail. Left to tarn's default that
6+
// wait is 30s per query on the request path. SqlDriver now bounds it by default,
7+
// while leaving a host's own choice alone.
8+
9+
import { describe, it, expect, afterEach } from 'vitest';
10+
import net from 'node:net';
11+
import { SqlDriver } from './sql-driver.js';
12+
13+
/** A listener that accepts sockets and then says nothing, ever. */
14+
function blackHole() {
15+
const held: net.Socket[] = [];
16+
const server = net.createServer((sock) => { sock.on('error', () => {}); held.push(sock); });
17+
return {
18+
listen: () => new Promise<number>((resolve) => {
19+
server.listen(0, '127.0.0.1', () => resolve((server.address() as net.AddressInfo).port));
20+
}),
21+
close: () => { held.forEach((s) => s.destroy()); server.close(); },
22+
};
23+
}
24+
25+
const drivers: SqlDriver[] = [];
26+
const make = (cfg: any) => { const d = new SqlDriver(cfg); drivers.push(d); return d; };
27+
28+
afterEach(async () => {
29+
await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {})));
30+
});
31+
32+
describe('SqlDriver — connection-attempt bound (framework#3769)', () => {
33+
it('applies a default createTimeoutMillis when the host sets no pool config', () => {
34+
const d = make({ client: 'pg', connection: 'postgres://u:p@127.0.0.1:1/d' });
35+
expect((d as any).knex.client.config.pool.createTimeoutMillis).toBe(10_000);
36+
});
37+
38+
it('applies it alongside a host pool config without clobbering min/max', () => {
39+
const d = make({
40+
client: 'pg',
41+
connection: 'postgres://u:p@127.0.0.1:1/d',
42+
pool: { min: 0, max: 5 },
43+
});
44+
const pool = (d as any).knex.client.config.pool;
45+
expect(pool).toMatchObject({ min: 0, max: 5, createTimeoutMillis: 10_000 });
46+
});
47+
48+
it("leaves a host's explicit createTimeoutMillis alone", () => {
49+
const d = make({
50+
client: 'pg',
51+
connection: 'postgres://u:p@127.0.0.1:1/d',
52+
pool: { min: 0, max: 5, createTimeoutMillis: 45_000 },
53+
});
54+
expect((d as any).knex.client.config.pool.createTimeoutMillis).toBe(45_000);
55+
});
56+
57+
it('actually bounds a query against a black-holing endpoint', async () => {
58+
const bh = blackHole();
59+
const port = await bh.listen();
60+
try {
61+
// 700ms so the assertion is decisive: unbounded is 30s.
62+
const d = make({
63+
client: 'pg',
64+
connection: `postgres://u:p@127.0.0.1:${port}/d`,
65+
pool: { min: 0, max: 2, createTimeoutMillis: 700 },
66+
});
67+
const started = Date.now();
68+
await expect((d as any).knex.raw('SELECT 1')).rejects.toThrow();
69+
expect(Date.now() - started).toBeLessThan(5_000);
70+
} finally {
71+
bh.close();
72+
}
73+
}, 20_000);
74+
75+
it('does not disturb a working sqlite connection', async () => {
76+
const d = make({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
77+
await expect(d.checkHealth()).resolves.toBe(true);
78+
});
79+
});

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,10 +579,42 @@ export class SqlDriver implements IDataDriver {
579579
this.schemaMode = schemaMode ?? 'managed';
580580
this.autoMigrate = autoMigrate ?? 'off';
581581
this.config = knexConfig;
582-
this.knex = knex(knexConfig);
582+
this.knex = knex(SqlDriver.withConnectBound(knexConfig));
583583
this.installQueryTiming();
584584
}
585585

586+
/**
587+
* Default bound on establishing ONE connection (framework#3769).
588+
*
589+
* A database endpoint that accepts the TCP connection but never completes the
590+
* handshake — an overloaded instance, a half-open firewall, a load balancer
591+
* mid-failover — is the failure mode that hurts most, because nothing fails:
592+
* the query just waits. Left to tarn's default that wait is **30 seconds**,
593+
* per query, on the request path; with a small `pool.max` a handful of them
594+
* saturate the pool and everything queues behind it.
595+
*
596+
* 10s matches driver-mongodb's existing `connectTimeoutMS ?? 10_000`, so both
597+
* drivers give up on an unreachable server at the same point. A host that
598+
* knows better (a deliberately slow cross-region replica) sets its own
599+
* `pool.createTimeoutMillis` and this leaves it alone.
600+
*
601+
* NOTE this bounds the wait but not the diagnosis: knex reports it as
602+
* "Timeout acquiring a connection. The pool is probably full", which points
603+
* at pool sizing rather than the network. Making the message accurate needs a
604+
* dialect-specific connect timeout (pg's `connectionTimeoutMillis`), which
605+
* changes the shape of `connection` — tracked in #3769.
606+
*/
607+
private static readonly DEFAULT_CREATE_TIMEOUT_MS = 10_000;
608+
609+
private static withConnectBound(knexConfig: Record<string, any>): Record<string, any> {
610+
const pool = knexConfig.pool as Record<string, any> | undefined;
611+
if (pool?.createTimeoutMillis !== undefined) return knexConfig; // host chose its own
612+
return {
613+
...knexConfig,
614+
pool: { ...(pool ?? {}), createTimeoutMillis: SqlDriver.DEFAULT_CREATE_TIMEOUT_MS },
615+
};
616+
}
617+
586618
/**
587619
* Per-request SQL query timing (perf-tuning mode). Correlates knex's
588620
* `query` → `query-response` / `query-error` events by `__knexQueryUid` and

packages/types/src/env.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,11 @@ export function resolveAllowDegradedTenancy(): boolean {
179179
*
180180
* Setting this to a truthy value (`true`/`1`/`on`/`yes`, case-insensitive)
181181
* boots anyway, in an explicitly degraded state that is logged loudly at
182-
* startup. There is NO reconnection: the drivers that failed stay dead for the
183-
* process lifetime and every query routed to them fails. Defaults OFF — an
184-
* unset flag means "fail fast".
182+
* startup. Every query routed to a failed driver fails until the datasource
183+
* becomes reachable — the underlying clients do re-establish connections on
184+
* their own (framework#3759) — but the boot-time schema sync those drivers
185+
* missed is never re-run, so their tables may simply not exist afterwards.
186+
* Defaults OFF — an unset flag means "fail fast".
185187
*/
186188
export function resolveAllowDriverConnectFailure(): boolean {
187189
const raw = readEnvWithDeprecation('OS_ALLOW_DRIVER_CONNECT_FAILURE', [], { silent: true });

0 commit comments

Comments
 (0)