Skip to content

Commit 1a5bc63

Browse files
committed
fix(driver-sql): pre-create autonumber sequences table to avoid batch/tx deadlock
Lazy creation of _objectstack_sequences on the first autonumber INSERT asked the pool for a second connection. Inside a /api/v1/batch transaction on SQLite (pool max=1) the only connection is already held, so the acquire blocked until 'Knex: Timeout acquiring a connection'. Postgres/MySQL hit the same pool exhaustion under concurrent cold first-writes. - initObjects now pre-creates the counter table outside any data transaction. - The lazy fallback runs its DDL on the caller's transaction on SQLite only (never on MySQL, where DDL would implicitly commit the caller's transaction). - Regression test reproduces the deadlock (fails on old code, passes now).
1 parent 1f32204 commit 1a5bc63

3 files changed

Lines changed: 202 additions & 20 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
---
4+
5+
Fix a connection-pool deadlock when the first `auto_number` write after process
6+
start goes through a transaction (e.g. `POST /api/v1/batch`, which wraps every
7+
operation in one `ql.transaction(...)`).
8+
9+
The sequence-counter table (`_objectstack_sequences`) was created lazily on the
10+
first autonumber INSERT via a bare `this.knex.schema.*` call that asks the pool
11+
for a second connection. On SQLite (better-sqlite3, pool max=1) the open batch
12+
transaction already holds the only connection, so the acquire blocked until
13+
`Knex: Timeout acquiring a connection`. Postgres/MySQL are exposed to the same
14+
pool-exhaustion deadlock under concurrent cold first-writes.
15+
16+
Fixes:
17+
- `initObjects` now pre-creates the counter table up front, outside any data
18+
transaction, so the first write never runs DDL (primary fix).
19+
- The lazy fallback (`ensureSequencesTable`) now runs its DDL on the caller's own
20+
transaction on SQLite instead of grabbing a second connection. It deliberately
21+
does not route DDL through the caller's transaction on MySQL, where DDL would
22+
implicitly commit the caller's in-flight transaction.
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Regression guard for the auto_number-in-transaction deadlock.
4+
//
5+
// Reported: on SQLite (better-sqlite3, pool max=1), the first autonumber write
6+
// after process start that goes through a transaction — e.g. POST /api/v1/batch,
7+
// which wraps every operation in one `ql.transaction(...)` — dead-locked with
8+
// "Knex: Timeout acquiring a connection. The pool is probably full."
9+
//
10+
// Root cause: the sequence-counter table (`_objectstack_sequences`) was created
11+
// lazily on the first autonumber INSERT, via a bare `this.knex.schema.*` call
12+
// that asks the pool for a SECOND connection. Inside a batch transaction the
13+
// only pooled connection is already held, so the acquire blocked until timeout.
14+
//
15+
// Fixes under test:
16+
// 1. `initObjects` pre-creates the table outside any data transaction, so the
17+
// first write never runs DDL (primary fix — covers the real batch path).
18+
// 2. The lazy fallback (`ensureSequencesTable`) runs its DDL on the caller's
19+
// own transaction on SQLite instead of grabbing a second connection, so
20+
// even a cold-cache in-transaction first write cannot deadlock.
21+
22+
import { describe, it, expect, afterEach } from 'vitest';
23+
import { SqlDriver } from '../src/index.js';
24+
25+
const SEQUENCES_TABLE = '_objectstack_sequences';
26+
27+
function withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> {
28+
return Promise.race([
29+
p,
30+
new Promise<T>((_, rej) => setTimeout(() => rej(new Error(`TIMEOUT: ${label} (deadlock)`)), ms)),
31+
]);
32+
}
33+
34+
describe('SqlDriver auto_number in-transaction (deadlock regression)', () => {
35+
let driver: SqlDriver | undefined;
36+
37+
afterEach(async () => {
38+
if (driver) await driver.disconnect();
39+
driver = undefined;
40+
});
41+
42+
async function setup() {
43+
driver = new SqlDriver({
44+
client: 'better-sqlite3',
45+
connection: { filename: ':memory:' },
46+
useNullAsDefault: true,
47+
});
48+
await driver.initObjects([
49+
{
50+
name: 'contract',
51+
fields: {
52+
organization_id: { type: 'string' },
53+
contract_number: { type: 'autonumber', format: 'CTR-{0000}' },
54+
name: { type: 'string' },
55+
},
56+
},
57+
]);
58+
return (driver as any).knex;
59+
}
60+
61+
it('pre-creates the sequences table during initObjects (before any write)', async () => {
62+
const k = await setup();
63+
// The counter table must exist immediately after initObjects, without a
64+
// single create() — this is what keeps the first write off the DDL path.
65+
expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
66+
expect((driver as any).sequencesTableReady).toBe(true);
67+
});
68+
69+
it('commits a batch-style transaction whose FIRST write fills an autonumber', async () => {
70+
const k = await setup();
71+
72+
// Two autonumber creates inside a single transaction, mirroring how the REST
73+
// /batch endpoint wraps operations. Must commit, not hang.
74+
const trx = await driver!.beginTransaction();
75+
const r1 = await withTimeout(
76+
driver!.create('contract', { organization_id: 'org_x', name: 'A' }, { transaction: trx } as any),
77+
6000,
78+
'batch create #1 (autonumber)',
79+
);
80+
const r2 = await withTimeout(
81+
driver!.create('contract', { organization_id: 'org_x', name: 'B' }, { transaction: trx } as any),
82+
6000,
83+
'batch create #2 (autonumber)',
84+
);
85+
await withTimeout(driver!.commit(trx), 6000, 'commit');
86+
87+
expect(r1.contract_number).toBe('CTR-0001');
88+
expect(r2.contract_number).toBe('CTR-0002');
89+
90+
const rows = await k('contract').select('contract_number').orderBy('contract_number');
91+
expect(rows.map((r: any) => r.contract_number)).toEqual(['CTR-0001', 'CTR-0002']);
92+
});
93+
94+
it('does not deadlock even with a COLD cache inside the transaction (lazy fallback)', async () => {
95+
const k = await setup();
96+
97+
// Simulate the path where the table was NOT pre-created (an external object,
98+
// or a consumer that writes without initObjects): drop it and clear the
99+
// process cache, then take the single connection with a transaction and let
100+
// the first autonumber write hit the lazy ensure. On the old code this asked
101+
// the pool for a second connection and blocked until acquire-timeout.
102+
await k.schema.dropTableIfExists(SEQUENCES_TABLE);
103+
(driver as any).sequencesTableReady = false;
104+
(driver as any).sequencesHasKeyHash = false;
105+
(driver as any).sequencesTableEnsurePromise = null;
106+
107+
const trx = await driver!.beginTransaction();
108+
const r1 = await withTimeout(
109+
driver!.create('contract', { organization_id: 'org_cold', name: 'C' }, { transaction: trx } as any),
110+
6000,
111+
'cold-cache create inside tx',
112+
);
113+
await withTimeout(driver!.commit(trx), 6000, 'commit');
114+
115+
expect(r1.contract_number).toBe('CTR-0001');
116+
// The table the fallback created on the transaction survives the commit.
117+
expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
118+
});
119+
});

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

Lines changed: 61 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -739,29 +739,42 @@ export class SqlDriver implements IDataDriver {
739739
* generous non-indexed column. Fixed-prefix formats use the empty scope and
740740
* keep their single global counter (backward compatible).
741741
*/
742-
protected async ensureSequencesTable(): Promise<void> {
742+
protected async ensureSequencesTable(parentTrx?: Knex.Transaction): Promise<void> {
743743
if (this.sequencesTableReady) return;
744744
if (this.sequencesTableEnsurePromise) {
745745
await this.sequencesTableEnsurePromise;
746746
return;
747747
}
748+
// Which connection runs the DDL below. Normally a fresh pooled connection
749+
// (`this.knex`), because `initObjects` pre-creates the table outside any data
750+
// transaction. This lazy path is the fallback (e.g. an external object, or a
751+
// consumer that writes without `initObjects`). If we are already inside the
752+
// caller's transaction AND the pool can only ever hand out one connection
753+
// (SQLite, pool max=1), that connection is busy with the open transaction —
754+
// a bare `this.knex` here would block forever acquiring a second one and then
755+
// fail with a Knex acquire-timeout (the reported batch/autonumber deadlock).
756+
// Run the DDL on the caller's own transaction instead; SQLite permits DDL
757+
// inside a transaction. We deliberately do NOT route DDL through `parentTrx`
758+
// on MySQL, where DDL implicitly commits the caller's transaction; there the
759+
// roomy pool (max=10) lets a fresh connection create the table safely.
760+
const runner: Knex | Knex.Transaction = parentTrx && this.isSqlite ? parentTrx : this.knex;
748761
this.sequencesTableEnsurePromise = (async () => {
749-
const exists = await this.knex.schema.hasTable(SEQUENCES_TABLE);
762+
const exists = await runner.schema.hasTable(SEQUENCES_TABLE);
750763
if (!exists) {
751764
try {
752-
await this.createSequencesTable(SEQUENCES_TABLE);
765+
await this.createSequencesTable(SEQUENCES_TABLE, runner);
753766
this.sequencesHasKeyHash = true;
754767
} catch (err: any) {
755768
// Race or cross-process create — re-check existence; ignore
756769
// "already exists" errors from any dialect.
757-
const stillMissing = !(await this.knex.schema.hasTable(SEQUENCES_TABLE));
770+
const stillMissing = !(await runner.schema.hasTable(SEQUENCES_TABLE));
758771
if (stillMissing) throw err;
759772
// A racing creator may have used an older schema. Migrate in place.
760-
await this.ensureSequencesKeyHashShape();
773+
await this.ensureSequencesKeyHashShape(runner);
761774
}
762775
} else {
763776
// Pre-existing table may predate the `key_hash`/`scope` shape. Migrate.
764-
await this.ensureSequencesKeyHashShape();
777+
await this.ensureSequencesKeyHashShape(runner);
765778
}
766779
this.sequencesTableReady = true;
767780
})();
@@ -779,9 +792,16 @@ export class SqlDriver implements IDataDriver {
779792
.digest('hex');
780793
}
781794

782-
/** Create the current `key_hash`-keyed sequences table shape. */
783-
protected async createSequencesTable(table: string): Promise<void> {
784-
await this.knex.schema.createTable(table, (t) => {
795+
/**
796+
* Create the current `key_hash`-keyed sequences table shape. `runner` is the
797+
* connection the DDL runs on (a fresh pooled connection by default, or the
798+
* caller's transaction on SQLite — see {@link ensureSequencesTable}).
799+
*/
800+
protected async createSequencesTable(
801+
table: string,
802+
runner: Knex | Knex.Transaction = this.knex,
803+
): Promise<void> {
804+
await runner.schema.createTable(table, (t) => {
785805
t.string('key_hash', 64).notNullable().primary();
786806
t.string('object').notNullable();
787807
t.string('tenant_id').notNullable();
@@ -806,17 +826,19 @@ export class SqlDriver implements IDataDriver {
806826
* sequences keep working via the legacy key and per-scope writes error
807827
* actionably (see getNextSequenceValue), rather than corrupting data.
808828
*/
809-
protected async ensureSequencesKeyHashShape(): Promise<void> {
810-
if (await this.knex.schema.hasColumn(SEQUENCES_TABLE, 'key_hash')) {
829+
protected async ensureSequencesKeyHashShape(
830+
runner: Knex | Knex.Transaction = this.knex,
831+
): Promise<void> {
832+
if (await runner.schema.hasColumn(SEQUENCES_TABLE, 'key_hash')) {
811833
this.sequencesHasKeyHash = true;
812834
return;
813835
}
814-
const hasScope = await this.knex.schema.hasColumn(SEQUENCES_TABLE, 'scope');
836+
const hasScope = await runner.schema.hasColumn(SEQUENCES_TABLE, 'scope');
815837
const TMP = `${SEQUENCES_TABLE}__rebuild`;
816838
try {
817-
const rows: any[] = await this.knex(SEQUENCES_TABLE).select('*');
818-
await this.knex.schema.dropTableIfExists(TMP);
819-
await this.createSequencesTable(TMP);
839+
const rows: any[] = await runner(SEQUENCES_TABLE).select('*');
840+
await runner.schema.dropTableIfExists(TMP);
841+
await this.createSequencesTable(TMP, runner);
820842
const migrated = rows.map((r) => {
821843
const scope = hasScope && r.scope != null ? String(r.scope) : '';
822844
return {
@@ -829,15 +851,15 @@ export class SqlDriver implements IDataDriver {
829851
updated_at: r.updated_at ?? this.knex.fn.now(),
830852
};
831853
});
832-
if (migrated.length > 0) await this.knex(TMP).insert(migrated);
833-
await this.knex.schema.dropTable(SEQUENCES_TABLE);
834-
await this.knex.schema.renameTable(TMP, SEQUENCES_TABLE);
854+
if (migrated.length > 0) await runner(TMP).insert(migrated);
855+
await runner.schema.dropTable(SEQUENCES_TABLE);
856+
await runner.schema.renameTable(TMP, SEQUENCES_TABLE);
835857
this.sequencesHasKeyHash = true;
836858
} catch (err) {
837859
// Leave the original table intact; fall back to legacy keying for
838860
// fixed-prefix sequences and refuse per-scope writes until migrated.
839861
this.sequencesHasKeyHash = false;
840-
await this.knex.schema.dropTableIfExists(TMP).catch(() => {});
862+
await runner.schema.dropTableIfExists(TMP).catch(() => {});
841863
this.logger.warn(
842864
`[autonumber] Failed to migrate ${SEQUENCES_TABLE} to the key_hash shape. ` +
843865
`Fixed-prefix autonumbers keep working; date/{field}/per-parent formats will ` +
@@ -902,7 +924,11 @@ export class SqlDriver implements IDataDriver {
902924
parentTrx?: Knex.Transaction,
903925
scope = '',
904926
): Promise<number> {
905-
await this.ensureSequencesTable();
927+
// Pass the caller's transaction so a cold-cache first write inside a batch
928+
// transaction ensures the table on the right connection instead of dead-
929+
// locking on a second one (SQLite pool max=1). `initObjects` normally warms
930+
// this up front, making the call a no-op — this only bites the lazy path.
931+
await this.ensureSequencesTable(parentTrx);
906932
const resolvedTenantId = tenantField && tenantId ? String(tenantId) : GLOBAL_TENANT;
907933
if (scope !== '' && !this.sequencesHasKeyHash) {
908934
// The legacy sequences table could not be migrated to the key_hash shape,
@@ -1720,6 +1746,21 @@ export class SqlDriver implements IDataDriver {
17201746
await this.reconcileAndWarnDrift(tableName, obj.fields ?? {});
17211747
}
17221748
}
1749+
1750+
// Pre-create the auto_number counter table now, while we hold a fresh pooled
1751+
// connection and are NOT inside any data transaction. Creating it lazily on
1752+
// the first autonumber INSERT dead-locks a `/api/v1/batch` write on SQLite
1753+
// (pool max=1: the open batch transaction owns the only connection, so the
1754+
// lazy `ensureSequencesTable` blocks forever acquiring a second one) and
1755+
// risks the same pool exhaustion under concurrent first-writes on
1756+
// Postgres/MySQL. Idempotent and skipped entirely when nothing uses
1757+
// auto_number, so it costs one `hasTable` at boot in the common case.
1758+
const usesAutoNumber = Object.values(this.autoNumberFields).some(
1759+
(cols) => Array.isArray(cols) && cols.length > 0,
1760+
);
1761+
if (usesAutoNumber) {
1762+
await this.ensureSequencesTable();
1763+
}
17231764
}
17241765

17251766
// ── Managed-schema drift & reconcile (#2186) ───────────────────────────────

0 commit comments

Comments
 (0)