Skip to content

Commit 21420d9

Browse files
os-zhuangclaude
andauthored
perf(seed,import): route bulk writes through the engine's batch insert path (#2680)
* perf(seed,import): route bulk writes through the engine's batch insert path Seed loader and data-import wrote records one at a time even though the engine's array-form insert() already does the efficient thing (one driver.bulkCreate round-trip + parent-deduplicated summary recompute). Add a shared bulkWrite/withTransientRetry helper (@objectstack/core) that batches writes, retries transient driver errors with backoff, and degrades to per-row writes when a batch fails for a non-transient reason so one bad row can't drop the rest. Wire it into SeedLoaderService.loadDataset() and import-runner's runImport(), and fix Protocol.createManyData to forward context to engine.insert() like createData already does. Fixes #2678. * fix(driver-sql): bulkCreate must assign a client-side id like create() does SqlDriver.bulkCreate() never ran the id/_id normalization create() applies, so a row inserted via the array path without a driver-native id default landed with id: NULL. This bulk path was rarely exercised before #2678 routed real seed/import data through it at scale, surfacing the gap (the showcase invoice-line dogfood test failed: product/invoice references resolved to NULL because the referenced object's bulk-inserted rows had no id to record). bulkCreate() now assigns nanoid(DEFAULT_ID_LENGTH) per row exactly like create() does. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6cebf22 commit 21420d9

12 files changed

Lines changed: 842 additions & 62 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/core': minor
3+
'@objectstack/metadata-protocol': minor
4+
'@objectstack/rest': minor
5+
'@objectstack/driver-sql': patch
6+
---
7+
8+
Seed loader and data-import now route bulk writes through the engine's array-form `insert()` (one round-trip per batch, with parent-deduplicated summary recompute) instead of one `insert()`/`createData()` call per record, and both retry transient driver errors instead of silently dropping the row (#2678).
9+
10+
A new shared helper, `bulkWrite` (`@objectstack/core`), batches rows through a caller-supplied batch-write function, retries a whole-batch transient failure (network blip / timeout) with exponential backoff, and degrades to per-row writes (each itself retried) when a batch fails for a non-transient reason — so one bad row can't drop the other N-1. `withTransientRetry` wraps a single write (e.g. an update) with the same retry behavior.
11+
12+
- `SeedLoaderService.loadDataset()` (`@objectstack/metadata-protocol`) buffers insert-mode records and flushes them in batches of 200 via the engine's array `insert()`. Datasets with a self-referencing field (e.g. `employee.manager_id -> employee`) keep the historical per-record write path, since a later record may need an earlier one's freshly-assigned id.
13+
- `runImport()` (`@objectstack/rest`) buffers create-resolved rows and flushes them via `protocol.createManyData()` when the protocol supports it, falling back to the original per-row `createData()` call otherwise. `Protocol.createManyData` (`@objectstack/metadata-protocol`) now forwards `context` to `engine.insert()` like `createData` already did, so tenant-scoped bulk creates work correctly.
14+
15+
Previously, a 1000-row seed or import into an object with a rollup summary issued 1000+ round-trips and up to 1000 summary recomputes; a single transient network error on any one row silently dropped it with no retry (the 2026-07-06 HotCRM first-boot incident). A `bulkCreate`-capable driver now sees roughly `ceil(N/batch)` writes, and a transient error is retried before a row is ever reported as failed.
16+
17+
**Fix (`@objectstack/driver-sql`):** `SqlDriver.bulkCreate()` never generated a client-side id for a row missing one, unlike `create()` — a latent gap that this change is the first to exercise at scale (a bulk-inserted row without a driver-native id default silently landed with `id: NULL`). `bulkCreate()` now mirrors `create()`'s id/`_id` normalization per row.

packages/core/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ export * from './utils/env.js';
2626
// Export timezone-aware calendar utilities (ADR-0053 Phase 2)
2727
export * from './utils/datetime.js';
2828

29+
// Export the shared batched-write helper (framework#2678)
30+
export * from './utils/bulk-write.js';
31+
2932
// Export in-memory fallbacks for core-criticality services
3033
export * from './fallbacks/index.js';
3134

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi } from 'vitest';
4+
import { bulkWrite, defaultIsTransientError, withTransientRetry } from './bulk-write';
5+
6+
const noopSleep = async () => {};
7+
8+
describe('bulkWrite', () => {
9+
it('writes N rows in ceil(N/batchSize) batch calls, not N single-row calls', async () => {
10+
const rows = Array.from({ length: 250 }, (_, i) => ({ n: i }));
11+
const writeBatch = vi.fn(async (batch: { n: number }[]) => batch.map((r) => ({ id: `r${r.n}` })));
12+
const writeOne = vi.fn(async (row: { n: number }) => ({ id: `r${row.n}` }));
13+
14+
const results = await bulkWrite(rows, { batchSize: 100, writeBatch, writeOne, sleep: noopSleep });
15+
16+
expect(writeBatch).toHaveBeenCalledTimes(3); // ceil(250/100)
17+
expect(writeOne).not.toHaveBeenCalled();
18+
expect(results).toHaveLength(250);
19+
expect(results.every((r) => r.ok)).toBe(true);
20+
// Results stay correlated to original row order across batch boundaries.
21+
expect(results[0].record).toEqual({ id: 'r0' });
22+
expect(results[249].record).toEqual({ id: 'r249' });
23+
});
24+
25+
it('retries a whole-batch transient failure and does not degrade to per-row on success', async () => {
26+
let attempts = 0;
27+
const writeBatch = vi.fn(async (batch: { n: number }[]) => {
28+
attempts++;
29+
if (attempts === 1) throw new Error('fetch failed');
30+
return batch.map((r) => ({ id: `r${r.n}` }));
31+
});
32+
const writeOne = vi.fn(async () => ({ id: 'x' }));
33+
34+
const rows = [{ n: 1 }, { n: 2 }];
35+
const results = await bulkWrite(rows, { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
36+
37+
expect(writeBatch).toHaveBeenCalledTimes(2);
38+
expect(writeOne).not.toHaveBeenCalled();
39+
expect(results.every((r) => r.ok)).toBe(true);
40+
});
41+
42+
it('degrades to per-row on a logical (non-transient) batch error, without failing the other rows', async () => {
43+
const writeBatch = vi.fn(async () => {
44+
throw new Error('CHECK constraint failed: score >= 0');
45+
});
46+
const writeOne = vi.fn(async (row: { n: number; bad?: boolean }) => {
47+
if (row.bad) throw new Error('CHECK constraint failed: score >= 0');
48+
return { id: `r${row.n}` };
49+
});
50+
51+
const rows = [{ n: 1 }, { n: 2, bad: true }, { n: 3 }];
52+
const results = await bulkWrite(rows, { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
53+
54+
expect(writeBatch).toHaveBeenCalledTimes(1);
55+
expect(writeOne).toHaveBeenCalledTimes(3);
56+
expect(results[0]).toMatchObject({ index: 0, ok: true, record: { id: 'r1' } });
57+
expect(results[1].ok).toBe(false);
58+
expect(results[1].error).toBeInstanceOf(Error);
59+
expect(results[2]).toMatchObject({ index: 2, ok: true, record: { id: 'r3' } });
60+
});
61+
62+
it('exhausts batch retries on a persistent transient error, then still degrades to per-row', async () => {
63+
const writeBatch = vi.fn(async () => {
64+
throw new Error('ETIMEDOUT');
65+
});
66+
const writeOne = vi.fn(async (row: { n: number }) => ({ id: `r${row.n}` }));
67+
68+
const rows = [{ n: 1 }, { n: 2 }];
69+
const results = await bulkWrite(rows, {
70+
batchSize: 10, maxRetries: 2, writeBatch, writeOne, sleep: noopSleep,
71+
});
72+
73+
expect(writeBatch).toHaveBeenCalledTimes(2); // maxRetries batch attempts
74+
expect(writeOne).toHaveBeenCalledTimes(2); // then one call per row
75+
expect(results.every((r) => r.ok)).toBe(true);
76+
});
77+
78+
it('keeps per-row index correlation intact across multiple batches with a mid-stream failure', async () => {
79+
const writeBatch = vi.fn(async (batch: { n: number; bad?: boolean }[]) => {
80+
if (batch.some((r) => r.bad)) throw new Error('validation error');
81+
return batch.map((r) => ({ id: `r${r.n}` }));
82+
});
83+
const writeOne = vi.fn(async (row: { n: number; bad?: boolean }) => {
84+
if (row.bad) throw new Error('validation error');
85+
return { id: `r${row.n}` };
86+
});
87+
88+
const rows = [{ n: 0 }, { n: 1 }, { n: 2, bad: true }, { n: 3 }, { n: 4 }];
89+
const results = await bulkWrite(rows, { batchSize: 2, writeBatch, writeOne, sleep: noopSleep });
90+
91+
expect(results.map((r) => r.index)).toEqual([0, 1, 2, 3, 4]);
92+
expect(results[2].ok).toBe(false);
93+
expect(results.filter((r) => r.ok)).toHaveLength(4);
94+
});
95+
96+
it('never retries a logical error at the row level either', async () => {
97+
const writeBatch = vi.fn(async () => { throw new Error('logical'); });
98+
const writeOne = vi.fn(async () => { throw new Error('NOT NULL constraint failed'); });
99+
100+
const results = await bulkWrite([{ n: 1 }, { n: 2 }], { batchSize: 10, maxRetries: 3, writeBatch, writeOne, sleep: noopSleep });
101+
102+
expect(writeOne).toHaveBeenCalledTimes(2); // no retry per row — not transient
103+
expect(results.every((r) => !r.ok)).toBe(true);
104+
});
105+
106+
it('a single-row batch failure is the row result directly, without a redundant writeOne call', async () => {
107+
const writeBatch = vi.fn(async () => { throw new Error('logical'); });
108+
const writeOne = vi.fn(async () => ({ id: 'should-not-be-called' }));
109+
110+
const results = await bulkWrite([{ n: 1 }], { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
111+
112+
expect(writeOne).not.toHaveBeenCalled();
113+
expect(results[0]).toMatchObject({ index: 0, ok: false });
114+
});
115+
});
116+
117+
describe('withTransientRetry', () => {
118+
it('retries a transient failure and returns the eventual success', async () => {
119+
let attempts = 0;
120+
const fn = vi.fn(async () => {
121+
attempts++;
122+
if (attempts < 3) throw new Error('fetch failed');
123+
return 'ok';
124+
});
125+
126+
const result = await withTransientRetry(fn, { sleep: noopSleep });
127+
128+
expect(result).toBe('ok');
129+
expect(fn).toHaveBeenCalledTimes(3);
130+
});
131+
132+
it('does not retry a logical error', async () => {
133+
const fn = vi.fn(async () => { throw new Error('UNIQUE constraint failed'); });
134+
135+
await expect(withTransientRetry(fn, { sleep: noopSleep })).rejects.toThrow('UNIQUE constraint failed');
136+
expect(fn).toHaveBeenCalledTimes(1);
137+
});
138+
});
139+
140+
describe('defaultIsTransientError', () => {
141+
it('recognizes common transient network signatures', () => {
142+
expect(defaultIsTransientError(new Error('fetch failed'))).toBe(true);
143+
expect(defaultIsTransientError(new Error('request timed out after 30000ms'))).toBe(true);
144+
expect(defaultIsTransientError(new Error('socket hang up'))).toBe(true);
145+
expect(defaultIsTransientError(Object.assign(new Error('connect ECONNRESET'), { code: 'ECONNRESET' }))).toBe(true);
146+
expect(defaultIsTransientError(new Error('Service returned 503'))).toBe(true);
147+
});
148+
149+
it('does not classify validation/constraint errors as transient', () => {
150+
expect(defaultIsTransientError(new Error('NOT NULL constraint failed: task.title'))).toBe(false);
151+
expect(defaultIsTransientError(new Error('Validation failed: email is required'))).toBe(false);
152+
expect(defaultIsTransientError(new Error('UNIQUE constraint failed: task.id'))).toBe(false);
153+
});
154+
});
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `bulkWrite` — the shared batched-write helper used by BOTH the seed loader
5+
* (`@objectstack/metadata-protocol`) and the data-import runner
6+
* (`@objectstack/rest`), so neither reimplements batching, transient-error
7+
* retry, or per-row degradation. See framework#2678.
8+
*
9+
* ObjectQL's engine already does the efficient thing when handed an ARRAY —
10+
* one `driver.bulkCreate` round-trip plus parent-deduplicated summary
11+
* recompute (`engine.insert(object, rows[])`) — but seed/import fed it one
12+
* record at a time, so neither got the benefit. This module re-chunks rows
13+
* into batches and drives them through a caller-supplied batch-write
14+
* function, adding:
15+
*
16+
* - transient-error retry (network blip / timeout) with exponential
17+
* backoff, so a dropped connection doesn't silently drop the row (the
18+
* 2026-07-06 HotCRM incident: a turso `fetch failed` mid-seed dropped rows
19+
* silently because nothing retried);
20+
* - per-row degradation when a batch fails for a non-transient (logical /
21+
* validation) reason, so one bad row can't fail the other N-1 — needed
22+
* because `driver.bulkCreate` is a single multi-row statement/`Promise.all`
23+
* on every driver in this repo (sql, memory, mongodb): one bad row fails
24+
* the whole call;
25+
* - a stable per-row result keyed by the row's original index, so callers
26+
* can reassemble output in input order even though rows are processed in
27+
* batches (and a batch's flush may be interleaved with other, immediate,
28+
* per-row work such as updates).
29+
*/
30+
31+
export interface BulkWriteRowResult<TRecord = any> {
32+
/** Index into the original `rows` array passed to {@link bulkWrite}. */
33+
index: number;
34+
ok: boolean;
35+
record?: TRecord;
36+
error?: unknown;
37+
}
38+
39+
export interface RetryOptions {
40+
/** Max attempts for one write (batch or single-row), including the first. Default 3. */
41+
maxRetries?: number;
42+
/** Base backoff in ms; doubled each retry, plus jitter. Default 200. */
43+
backoffBaseMs?: number;
44+
/** Classifies an error as transient (worth retrying) vs logical (the row/batch is just bad). */
45+
isTransientError?: (err: unknown) => boolean;
46+
/** Injectable sleep, for deterministic tests. */
47+
sleep?: (ms: number) => Promise<void>;
48+
}
49+
50+
export interface BulkWriteOptions<TRow, TRecord = any> extends RetryOptions {
51+
/** Rows per batch. Default 200 (framework#2678 suggests 100-500). */
52+
batchSize?: number;
53+
/**
54+
* Write one batch. MUST resolve to one record per input row, in the SAME
55+
* order as `batch` — {@link bulkWrite} correlates `records[i]` back to
56+
* `batch[i]` positionally (this is how every `bulkCreate` implementation in
57+
* this repo already behaves: sql's single `INSERT ... VALUES (...), (...)
58+
* RETURNING *`, memory's `Promise.all`, mongodb's ordered `insertMany`).
59+
*/
60+
writeBatch: (batch: TRow[]) => Promise<TRecord[]>;
61+
/** Write a single row — used only to degrade a failed batch. */
62+
writeOne: (row: TRow) => Promise<TRecord>;
63+
}
64+
65+
const DEFAULT_BATCH_SIZE = 200;
66+
const DEFAULT_MAX_RETRIES = 3;
67+
const DEFAULT_BACKOFF_BASE_MS = 200;
68+
69+
/**
70+
* Transient-error signatures shared by common HTTP/TCP-backed drivers
71+
* (turso/libsql's fetch-based transport included). Deliberately excludes
72+
* anything that looks like a validation/constraint error — those must NOT be
73+
* retried, only degraded to per-row.
74+
*/
75+
const TRANSIENT_PATTERNS: RegExp[] = [
76+
/fetch failed/i,
77+
/network/i,
78+
/timed?\s*out/i,
79+
/timeout/i,
80+
/socket hang ?up/i,
81+
/connection.*(closed|reset|refused|terminated|aborted)/i,
82+
/\b(502|503|504)\b/,
83+
/server.*unavailable/i,
84+
/too many connections/i,
85+
];
86+
87+
const TRANSIENT_CODES = /^(ECONNRESET|ECONNREFUSED|ECONNABORTED|EPIPE|EAI_AGAIN|ETIMEDOUT|EHOSTUNREACH|ENETUNREACH|ENOTFOUND)$/i;
88+
89+
export function defaultIsTransientError(err: unknown): boolean {
90+
const code = (err as { code?: unknown } | null)?.code;
91+
if (typeof code === 'string' && TRANSIENT_CODES.test(code)) return true;
92+
const message = (err as { message?: unknown } | null)?.message;
93+
const text = typeof message === 'string' ? message : String(err ?? '');
94+
return TRANSIENT_PATTERNS.some((re) => re.test(text));
95+
}
96+
97+
const defaultSleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
98+
99+
interface ResolvedRetryOptions {
100+
maxRetries: number;
101+
backoffBaseMs: number;
102+
isTransientError: (err: unknown) => boolean;
103+
sleep: (ms: number) => Promise<void>;
104+
}
105+
106+
async function withRetry<T>(fn: () => Promise<T>, opts: ResolvedRetryOptions): Promise<T> {
107+
let lastError: unknown;
108+
for (let attempt = 1; attempt <= opts.maxRetries; attempt++) {
109+
try {
110+
return await fn();
111+
} catch (err) {
112+
lastError = err;
113+
if (attempt >= opts.maxRetries || !opts.isTransientError(err)) throw err;
114+
const jitter = Math.floor(Math.random() * 50);
115+
await opts.sleep(opts.backoffBaseMs * 2 ** (attempt - 1) + jitter);
116+
}
117+
}
118+
// Unreachable — the loop above always returns or throws — but keeps TS's
119+
// control-flow analysis happy about a guaranteed return type.
120+
throw lastError;
121+
}
122+
123+
/**
124+
* Retry a single write (e.g. an `engine.update()` call the seed loader or
125+
* import runner makes outside the batched-insert path) with the same
126+
* transient-error backoff {@link bulkWrite} applies to batches — so a
127+
* network blip doesn't drop an update the way it used to drop an insert.
128+
*/
129+
export async function withTransientRetry<T>(fn: () => Promise<T>, opts: RetryOptions = {}): Promise<T> {
130+
return withRetry(fn, {
131+
maxRetries: Math.max(1, opts.maxRetries ?? DEFAULT_MAX_RETRIES),
132+
backoffBaseMs: opts.backoffBaseMs ?? DEFAULT_BACKOFF_BASE_MS,
133+
isTransientError: opts.isTransientError ?? defaultIsTransientError,
134+
sleep: opts.sleep ?? defaultSleep,
135+
});
136+
}
137+
138+
/**
139+
* Write `rows` through `opts.writeBatch` in chunks of `opts.batchSize`,
140+
* retrying a whole-batch transient failure with backoff, and degrading to
141+
* per-row `opts.writeOne` calls (each itself retried) when a batch fails for
142+
* a non-transient reason — so one bad row can't drop the rest of the batch.
143+
*
144+
* Returns one {@link BulkWriteRowResult} per input row, indexed to match
145+
* `rows`' original order.
146+
*/
147+
export async function bulkWrite<TRow, TRecord = any>(
148+
rows: TRow[],
149+
opts: BulkWriteOptions<TRow, TRecord>,
150+
): Promise<BulkWriteRowResult<TRecord>[]> {
151+
const batchSize = Math.max(1, opts.batchSize ?? DEFAULT_BATCH_SIZE);
152+
const retryOpts: ResolvedRetryOptions = {
153+
maxRetries: Math.max(1, opts.maxRetries ?? DEFAULT_MAX_RETRIES),
154+
backoffBaseMs: opts.backoffBaseMs ?? DEFAULT_BACKOFF_BASE_MS,
155+
isTransientError: opts.isTransientError ?? defaultIsTransientError,
156+
sleep: opts.sleep ?? defaultSleep,
157+
};
158+
159+
const results: BulkWriteRowResult<TRecord>[] = new Array(rows.length);
160+
161+
for (let start = 0; start < rows.length; start += batchSize) {
162+
const batch = rows.slice(start, start + batchSize);
163+
try {
164+
const records = await withRetry(() => opts.writeBatch(batch), retryOpts);
165+
for (let i = 0; i < batch.length; i++) {
166+
results[start + i] = { index: start + i, ok: true, record: records[i] };
167+
}
168+
} catch (batchErr) {
169+
// A single-row "batch" already IS the per-row attempt — its failure
170+
// (after transient retry) is the row's final outcome; calling
171+
// `writeOne` again would just repeat the identical work.
172+
if (batch.length === 1) {
173+
results[start] = { index: start, ok: false, error: batchErr };
174+
continue;
175+
}
176+
// The batch failed even after transient retry, or failed for a logical
177+
// reason retry wouldn't fix. Degrade to per-row so one bad row can't
178+
// fail the other rows in this batch. Each row still gets its own
179+
// transient retry — the batch-level failure doesn't tell us which row
180+
// (if any) was actually the transient one.
181+
for (let i = 0; i < batch.length; i++) {
182+
const idx = start + i;
183+
try {
184+
const record = await withRetry(() => opts.writeOne(batch[i]), retryOpts);
185+
results[idx] = { index: idx, ok: true, record };
186+
} catch (err) {
187+
results[idx] = { index: idx, ok: false, error: err };
188+
}
189+
}
190+
}
191+
}
192+
193+
return results;
194+
}

packages/metadata-protocol/src/protocol.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,8 +2918,12 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
29182918
} as BatchUpdateResponse;
29192919
}
29202920

2921-
async createManyData(request: { object: string, records: any[] }): Promise<any> {
2922-
const records = await this.engine.insert(request.object, request.records);
2921+
async createManyData(request: { object: string, records: any[], context?: any }): Promise<any> {
2922+
const records = await this.engine.insert(
2923+
request.object,
2924+
request.records,
2925+
request.context !== undefined ? { context: request.context } as any : undefined,
2926+
);
29232927
return {
29242928
object: request.object,
29252929
records,

0 commit comments

Comments
 (0)