Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .changeset/driver-connect-bound-and-reconnect-correction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
"@objectstack/driver-sql": minor
"@objectstack/objectql": patch
"@objectstack/types": patch
---

fix(driver-sql): bound a connection attempt at 10s, and correct the "no reconnection" claim (#3769, #3759)

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 — "there is no lazy reconnection", "NOT retried and NOT
reconnected", "stays disconnected for the process lifetime". Measured, both
drivers recover on their own:

- driver-mongodb: killing a real `mongod` and restarting it on the same port,
the *same* driver instance served the next write successfully (13ms), with no
reconnect call from us — the official driver's topology monitor handles it.
- driver-sql: a knex/pg pool is not poisoned by an outage. Its error tracks live
server state (`ECONNREFUSED` while down → a handshake error once a listener is
back → `ECONNREFUSED` again), i.e. every acquire opens a fresh connection.
`storage-driver.ts` also configures `pool.min: 0`, so no stale idle
connections are held.

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. The claims are now corrected in `DriverConnectError`,
the `DEGRADED BOOT` banner, `resolveAllowDriverConnectFailure`'s docs, and the
drivers / self-hosting pages.

**Fail-fast at boot is unchanged and still correct** — the reason is just
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 banner now says that.

**The real defect underneath.** `SqlDriver` passed its config to knex untouched,
so a database endpoint that accepts TCP but never completes the handshake — an
overloaded instance, a half-open firewall, a load balancer mid-failover — 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. With a small `pool.max` a few such queries saturate the
pool and everything else queues.

`SqlDriver` now defaults `pool.createTimeoutMillis` to **10s**, matching
driver-mongodb's existing `connectTimeoutMS ?? 10_000` so both drivers give up on
an unreachable server at the same point. A host that sets its own
`createTimeoutMillis` is left alone.

**Migration.** None for a healthy datasource. A deployment that deliberately
relies on connection establishment taking longer than 10s (a slow cross-region
replica) should set `pool.createTimeoutMillis` explicitly on its `SqlDriver`
config.

Not fixed here, tracked in #3769: knex still reports the bounded wait as "the
pool is probably full". An accurate message needs a dialect-specific connect
timeout (pg's `connectionTimeoutMillis`), which changes the shape of `connection`
and would regress the startup banner's URL display.
23 changes: 15 additions & 8 deletions content/docs/data-modeling/drivers.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,24 @@ bootstrap. If any `connect()` rejects, the boot is **refused**
([#3741](https://github.com/objectstack-ai/objectstack/issues/3741)) — the error
names each failed driver and its cause, and `objectstack serve` exits 1.

There is no lazy reconnection. A driver that did not connect at startup stays
disconnected for the process lifetime, so booting anyway would produce a server
that reports itself started, answers health checks, and then fails every request
with an error nothing like *the database is unreachable* — while the schema sync
that follows `init()` issues DDL against a datasource that isn't there.
Booting without a reachable datasource would produce a server that reports
itself started, answers health checks, and then fails every request with an
error nothing like *the database is unreachable* — while the schema sync that
follows `init()` issues DDL against a datasource that isn't there.

The reason is **not** that the connection can never come back. Client libraries
do re-establish connections on their own — the MongoDB driver's topology monitor
reconnects, and knex/pg opens a fresh connection per acquire (verified in
[#3759](https://github.com/objectstack-ai/objectstack/issues/3759)). What never
comes back is the **boot sequence that was skipped**: nothing re-runs the schema
sync, so those objects can be left with no tables even after the database
returns.

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

### Writing a driver: `connect()` is where you refuse to start
Expand Down
15 changes: 8 additions & 7 deletions content/docs/deployment/self-hosting.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -298,13 +298,14 @@ cleanly and a replica that lost its database stops receiving traffic instead of
serving 500s. Before setting `replicas > 1`, read the multi-node note below.

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

## Reverse proxy & TLS
Expand Down
8 changes: 4 additions & 4 deletions packages/objectql/src/driver-connect-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ export class DriverConnectError extends Error {
super(
`${failures.length} of ${totalDrivers} data driver(s) failed to connect — refusing to boot.\n` +
`${detail}\n` +
`A driver that did not connect cannot serve queries (there is no lazy reconnection) and the ` +
`schema sync that runs right after init would issue DDL against it. Fix the datasource ` +
`configuration (e.g. OS_DATABASE_URL), or set OS_ALLOW_DRIVER_CONNECT_FAILURE=1 to boot anyway ` +
`in an explicitly degraded state where every query to those drivers fails.`,
`A driver that did not connect cannot serve queries, and the schema sync that runs right ` +
`after init would issue DDL against it. Fix the datasource configuration (e.g. ` +
`OS_DATABASE_URL), or set OS_ALLOW_DRIVER_CONNECT_FAILURE=1 to boot anyway and serve errors ` +
`until the datasource becomes reachable.`,
);
this.name = 'DriverConnectError';
if (failures[0]?.error instanceof Error) {
Expand Down
4 changes: 3 additions & 1 deletion packages/objectql/src/engine-driver-connect-failfast.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ describe('ObjectQL.init() — driver connect fail-fast (framework#3741)', () =>
await expect(engine.init()).resolves.toBeUndefined();
const warned = warnings.join('\n');
expect(warned).toContain('DEGRADED BOOT');
expect(warned).toContain('NOT reconnected');
// The durable consequence is the SKIPPED SCHEMA SYNC, not a dead connection:
// the client reconnects on its own, but nothing re-runs the DDL (#3759).
expect(warned).toContain('SKIPPED FOR GOOD');
expect(warned).toContain('sql');
});

Expand Down
19 changes: 11 additions & 8 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1891,13 +1891,15 @@ export class ObjectQL implements IDataEngine {
* `connect()` rejects, this throws {@link DriverConnectError} and kernel
* bootstrap aborts. Two reasons, both load-bearing:
*
* 1. A driver that did not connect never recovers — there is no lazy
* reconnection anywhere in `driver-sql` / `driver-mongodb`. Booting
* anyway produces a server that reports itself started, may even answer
* health checks, and then 500s every request with an error that reads
* 1. Booting without a reachable datasource produces a server that reports
* itself started and then 500s every request with an error that reads
* nothing like "the database is unreachable". The caller immediately
* makes it worse: `ObjectQLPlugin.start()` runs `syncRegisteredSchemas()`
* right after this, issuing DDL against a driver that isn't there.
* right after this, issuing DDL against a driver that isn't there — so
* the boot leaves the schema half-applied even if the database appears
* a second later. Recovery is not the point: the underlying clients DO
* re-establish connections on their own (framework#3759 verified this),
* but nothing re-runs the boot sequence that was skipped.
* 2. Swallowing the rejection removed a driver's ability to REFUSE STARTUP.
* Any fatal startup check — licence, server version, incompatible
* configuration, missing capability, not just an unreachable socket — is
Expand Down Expand Up @@ -1935,9 +1937,10 @@ export class ObjectQL implements IDataEngine {
const banner =
`⚠️ DEGRADED BOOT: ${failures.length} of ${this.drivers.size} driver(s) failed to connect ` +
`(${failedDrivers.join(', ')}), but OS_ALLOW_DRIVER_CONNECT_FAILURE is set — starting anyway. ` +
`These drivers are NOT retried and NOT reconnected: every query and every schema sync routed ` +
`to them fails for the lifetime of this process. Unset OS_ALLOW_DRIVER_CONNECT_FAILURE to ` +
`restore fail-fast boot.`;
`Every query routed to them fails until the datasource becomes reachable, and the boot-time ` +
`schema sync is SKIPPED FOR GOOD — the client will reconnect on its own, but nothing re-runs ` +
`the DDL, so those objects may have no tables even after the database comes back. Unset ` +
`OS_ALLOW_DRIVER_CONNECT_FAILURE to restore fail-fast boot.`;
this.logger.warn(banner, { failedDrivers });
// …and again on a channel the host cannot silence — see the helper's note
// on `os serve`'s boot-quiet stdout capture.
Expand Down
79 changes: 79 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver-connect-bound.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
//
// framework#3769: a database endpoint that accepts the TCP connection but never
// completes the handshake (overloaded instance, half-open firewall, LB mid-
// failover) makes every query WAIT rather than fail. Left to tarn's default that
// wait is 30s per query on the request path. SqlDriver now bounds it by default,
// while leaving a host's own choice alone.

import { describe, it, expect, afterEach } from 'vitest';
import net from 'node:net';
import { SqlDriver } from './sql-driver.js';

/** A listener that accepts sockets and then says nothing, ever. */
function blackHole() {
const held: net.Socket[] = [];
const server = net.createServer((sock) => { sock.on('error', () => {}); held.push(sock); });
return {
listen: () => new Promise<number>((resolve) => {
server.listen(0, '127.0.0.1', () => resolve((server.address() as net.AddressInfo).port));
}),
close: () => { held.forEach((s) => s.destroy()); server.close(); },
};
}

const drivers: SqlDriver[] = [];
const make = (cfg: any) => { const d = new SqlDriver(cfg); drivers.push(d); return d; };

afterEach(async () => {
await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {})));
});

describe('SqlDriver — connection-attempt bound (framework#3769)', () => {
it('applies a default createTimeoutMillis when the host sets no pool config', () => {
const d = make({ client: 'pg', connection: 'postgres://u:p@127.0.0.1:1/d' });
expect((d as any).knex.client.config.pool.createTimeoutMillis).toBe(10_000);
});

it('applies it alongside a host pool config without clobbering min/max', () => {
const d = make({
client: 'pg',
connection: 'postgres://u:p@127.0.0.1:1/d',
pool: { min: 0, max: 5 },
});
const pool = (d as any).knex.client.config.pool;
expect(pool).toMatchObject({ min: 0, max: 5, createTimeoutMillis: 10_000 });
});

it("leaves a host's explicit createTimeoutMillis alone", () => {
const d = make({
client: 'pg',
connection: 'postgres://u:p@127.0.0.1:1/d',
pool: { min: 0, max: 5, createTimeoutMillis: 45_000 },
});
expect((d as any).knex.client.config.pool.createTimeoutMillis).toBe(45_000);
});

it('actually bounds a query against a black-holing endpoint', async () => {
const bh = blackHole();
const port = await bh.listen();
try {
// 700ms so the assertion is decisive: unbounded is 30s.
const d = make({
client: 'pg',
connection: `postgres://u:p@127.0.0.1:${port}/d`,
pool: { min: 0, max: 2, createTimeoutMillis: 700 },
});
const started = Date.now();
await expect((d as any).knex.raw('SELECT 1')).rejects.toThrow();
expect(Date.now() - started).toBeLessThan(5_000);
} finally {
bh.close();
}
}, 20_000);

it('does not disturb a working sqlite connection', async () => {
const d = make({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
await expect(d.checkHealth()).resolves.toBe(true);
});
});
34 changes: 33 additions & 1 deletion packages/plugins/driver-sql/src/sql-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,10 +569,42 @@ export class SqlDriver implements IDataDriver {
this.schemaMode = schemaMode ?? 'managed';
this.autoMigrate = autoMigrate ?? 'off';
this.config = knexConfig;
this.knex = knex(knexConfig);
this.knex = knex(SqlDriver.withConnectBound(knexConfig));
this.installQueryTiming();
}

/**
* Default bound on establishing ONE connection (framework#3769).
*
* A database endpoint that accepts the TCP connection but never completes the
* handshake — an overloaded instance, a half-open firewall, a load balancer
* mid-failover — is the failure mode that hurts most, because nothing fails:
* the query just waits. Left to tarn's default that wait is **30 seconds**,
* per query, on the request path; with a small `pool.max` a handful of them
* saturate the pool and everything queues behind it.
*
* 10s matches driver-mongodb's existing `connectTimeoutMS ?? 10_000`, so both
* drivers give up on an unreachable server at the same point. A host that
* knows better (a deliberately slow cross-region replica) sets its own
* `pool.createTimeoutMillis` and this leaves it alone.
*
* NOTE this bounds the wait but not the diagnosis: knex reports it as
* "Timeout acquiring a connection. The pool is probably full", which points
* at pool sizing rather than the network. Making the message accurate needs a
* dialect-specific connect timeout (pg's `connectionTimeoutMillis`), which
* changes the shape of `connection` — tracked in #3769.
*/
private static readonly DEFAULT_CREATE_TIMEOUT_MS = 10_000;

private static withConnectBound(knexConfig: Record<string, any>): Record<string, any> {
const pool = knexConfig.pool as Record<string, any> | undefined;
if (pool?.createTimeoutMillis !== undefined) return knexConfig; // host chose its own
return {
...knexConfig,
pool: { ...(pool ?? {}), createTimeoutMillis: SqlDriver.DEFAULT_CREATE_TIMEOUT_MS },
};
}

/**
* Per-request SQL query timing (perf-tuning mode). Correlates knex's
* `query` → `query-response` / `query-error` events by `__knexQueryUid` and
Expand Down
8 changes: 5 additions & 3 deletions packages/types/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,11 @@ export function resolveAllowDegradedTenancy(): boolean {
*
* Setting this to a truthy value (`true`/`1`/`on`/`yes`, case-insensitive)
* boots anyway, in an explicitly degraded state that is logged loudly at
* startup. There is NO reconnection: the drivers that failed stay dead for the
* process lifetime and every query routed to them fails. Defaults OFF — an
* unset flag means "fail fast".
* startup. Every query routed to a failed driver fails until the datasource
* becomes reachable — the underlying clients do re-establish connections on
* their own (framework#3759) — but the boot-time schema sync those drivers
* missed is never re-run, so their tables may simply not exist afterwards.
* Defaults OFF — an unset flag means "fail fast".
*/
export function resolveAllowDriverConnectFailure(): boolean {
const raw = readEnvWithDeprecation('OS_ALLOW_DRIVER_CONNECT_FAILURE', [], { silent: true });
Expand Down
Loading