mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
- preInitSqlJs() now memoizes an in-flight Promise (not just the resolved adapter) per filePath, so concurrent BATCH/STARTUP/HealthCheck/ ProviderLimitsSync callers at boot share one full-file read+WASM decode instead of each independently reloading the whole database — the thundering-herd amplifier of the OOM condition #6632 already partly fixed, left un-implemented by the reporter's own proposed fix (#6628). - sqljsAdapter's run/get/all now unwrap a lone named-parameter object (e.g. .all({ isActive: 1 }) for "WHERE is_active = @isActive", the same call shape getProviderConnections() already uses against better-sqlite3) before calling sql.js's stmt.bind(), expanding it to the @/:/$ sigil variants sql.js's own named-bind path requires. Previously the object was wrapped into an array and sql.js took the positional-bind path, throwing "Wrong API use : tried to bind a value of an unknown type ([object Object])." whenever the sql.js WASM fallback driver was active — exactly the error #6802 reported (misattributed to better-sqlite3). Regression tests added to tests/unit/db-adapters/driverFactory.test.ts and tests/unit/db-adapters/sqljsAdapter.test.ts, both proven RED against the prior code and GREEN after the fix.
This commit is contained in:
committed by
GitHub
parent
ac81235609
commit
ec553dd9c0
2
changelog.d/fixes/6628-6628-sqljs-adapter.md
Normal file
2
changelog.d/fixes/6628-6628-sqljs-adapter.md
Normal file
@@ -0,0 +1,2 @@
|
||||
- fix(db): share one in-flight sql.js load across concurrent `preInitSqlJs()` callers to stop the boot-time thundering-herd re-decode of the whole database file (#6628)
|
||||
- fix(db): unwrap lone named-parameter objects before `sql.js` `stmt.bind()` so `@`/`:`/`$`-style named placeholders bind correctly instead of throwing "Wrong API use" (#6802)
|
||||
@@ -10,6 +10,7 @@ const _require = createRequire(import.meta.url);
|
||||
|
||||
declare global {
|
||||
var __omnirouteSqlJsAdapters: Map<string, SqliteAdapter> | undefined;
|
||||
var __omnirouteSqlJsInitPromises: Map<string, Promise<SqliteAdapter>> | undefined;
|
||||
}
|
||||
|
||||
function getSqlJsCache(): Map<string, SqliteAdapter> {
|
||||
@@ -19,6 +20,20 @@ function getSqlJsCache(): Map<string, SqliteAdapter> {
|
||||
return globalThis.__omnirouteSqlJsAdapters;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cache das Promises de inicialização EM VOO (não resolvidas ainda), por filePath.
|
||||
* Separado de getSqlJsCache() (que só guarda o adapter já resolvido) para que
|
||||
* chamadores concorrentes (BATCH/STARTUP/HealthCheck/ProviderLimitsSync no boot)
|
||||
* compartilhem UMA única leitura+decode do arquivo em vez de cada um chamar
|
||||
* fs.readFileSync + WASM decode independentemente (#6628 — thundering herd).
|
||||
*/
|
||||
function getSqlJsPendingCache(): Map<string, Promise<SqliteAdapter>> {
|
||||
if (!globalThis.__omnirouteSqlJsInitPromises) {
|
||||
globalThis.__omnirouteSqlJsInitPromises = new Map();
|
||||
}
|
||||
return globalThis.__omnirouteSqlJsInitPromises;
|
||||
}
|
||||
|
||||
/** Tenta abrir com better-sqlite3 e node:sqlite sincronamente. Retorna null se ambos falharem. */
|
||||
export function tryOpenSync(
|
||||
filePath: string,
|
||||
@@ -75,10 +90,26 @@ export async function preInitSqlJs(filePath: string): Promise<SqliteAdapter> {
|
||||
cache.delete(filePath);
|
||||
}
|
||||
|
||||
const { createSqlJsAdapter } = await import("./sqljsAdapter");
|
||||
const adapter = await createSqlJsAdapter(filePath);
|
||||
cache.set(filePath, adapter);
|
||||
return adapter;
|
||||
// Share one in-flight load across concurrent callers for the same filePath
|
||||
// (#6628): without this, each of BATCH/STARTUP/HealthCheck/ProviderLimitsSync
|
||||
// independently fs.readFileSync + WASM-decode the same (possibly 300+MB) file
|
||||
// at boot, multiplying peak memory pressure by the number of racing callers.
|
||||
const pending = getSqlJsPendingCache();
|
||||
const inflight = pending.get(filePath);
|
||||
if (inflight) return inflight;
|
||||
|
||||
const initPromise = (async () => {
|
||||
const { createSqlJsAdapter } = await import("./sqljsAdapter");
|
||||
const adapter = await createSqlJsAdapter(filePath);
|
||||
cache.set(filePath, adapter);
|
||||
return adapter;
|
||||
})();
|
||||
pending.set(filePath, initPromise);
|
||||
try {
|
||||
return await initPromise;
|
||||
} finally {
|
||||
pending.delete(filePath);
|
||||
}
|
||||
}
|
||||
|
||||
/** Retorna adapter sql.js pré-inicializado ou null se ainda não inicializado. */
|
||||
|
||||
@@ -31,6 +31,57 @@ function resolveSqlJsWasmPath(): string {
|
||||
return candidatePaths[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* better-sqlite3's named-parameter convention lets callers bind with the bare
|
||||
* property name (e.g. `{ isActive: 1 }` for a SQL placeholder written as
|
||||
* `@isActive`, `:isActive`, or `$isActive` — better-sqlite3 strips the sigil
|
||||
* internally). sql.js's own named-bind path (`sqlite3_bind_parameter_index`)
|
||||
* requires the FULL name INCLUDING the sigil, and silently no-ops (does not
|
||||
* throw) for a key it can't resolve. Expand each bare key to all three
|
||||
* sigil-prefixed variants so sql.js matches whichever sigil the SQL actually
|
||||
* used, while passing through any key the caller already prefixed unchanged.
|
||||
*/
|
||||
function withNamedParamPrefixes(obj: Record<string, unknown>): Record<string, unknown> {
|
||||
const expanded: Record<string, unknown> = {};
|
||||
for (const [key, value] of Object.entries(obj)) {
|
||||
if (/^[:@$]/.test(key)) {
|
||||
expanded[key] = value;
|
||||
continue;
|
||||
}
|
||||
expanded[`@${key}`] = value;
|
||||
expanded[`:${key}`] = value;
|
||||
expanded[`$${key}`] = value;
|
||||
}
|
||||
return expanded;
|
||||
}
|
||||
|
||||
/**
|
||||
* sql.js's own `stmt.bind()` dispatches on shape: an Array means positional
|
||||
* bind (each element -> bind index N), a plain object means named-parameter
|
||||
* bind. Callers here always pass their rest-args as an array, so a caller
|
||||
* doing `.all({ isActive: 1 })` for a named placeholder (mirrors
|
||||
* better-sqlite3's spread-args named-bind convention, see
|
||||
* betterSqliteAdapter.ts) ends up handing sql.js `[{isActive:1}]` — an ARRAY
|
||||
* containing the object — which sql.js treats as a single positional value
|
||||
* and rejects with "Wrong API use : tried to bind a value of an unknown
|
||||
* type (...)." (#6802). Unwrap a lone plain-object param back to the object
|
||||
* itself (sigil-expanded) so sql.js takes its named-bind path instead.
|
||||
*/
|
||||
function toBindValue(params: unknown[]): unknown[] | Record<string, unknown> | undefined {
|
||||
if (!params.length) return undefined;
|
||||
const [first] = params;
|
||||
const isLoneNamedParamsObject =
|
||||
params.length === 1 &&
|
||||
first !== null &&
|
||||
typeof first === "object" &&
|
||||
!Array.isArray(first) &&
|
||||
!Buffer.isBuffer(first) &&
|
||||
!(first instanceof Uint8Array);
|
||||
return isLoneNamedParamsObject
|
||||
? withNamedParamPrefixes(first as Record<string, unknown>)
|
||||
: params;
|
||||
}
|
||||
|
||||
async function loadSqlJs(): Promise<typeof _sqlJsLib> {
|
||||
if (_sqlJsLib) return _sqlJsLib;
|
||||
const initSqlJs = ((await import("sql.js")) as { default: (typeof import("sql.js"))["default"] })
|
||||
@@ -103,7 +154,8 @@ export async function createSqlJsAdapter(filePath: string): Promise<SqliteAdapte
|
||||
run(...params: unknown[]): RunResult {
|
||||
const stmt = db.prepare(sql);
|
||||
try {
|
||||
if (params.length) stmt.bind(params as unknown[]);
|
||||
const bindValue = toBindValue(params);
|
||||
if (bindValue !== undefined) stmt.bind(bindValue);
|
||||
stmt.step();
|
||||
const changes = db.getRowsModified();
|
||||
const lastRows = db.exec("SELECT last_insert_rowid() as id");
|
||||
@@ -117,7 +169,8 @@ export async function createSqlJsAdapter(filePath: string): Promise<SqliteAdapte
|
||||
get(...params: unknown[]): unknown {
|
||||
const stmt = db.prepare(sql);
|
||||
try {
|
||||
if (params.length) stmt.bind(params as unknown[]);
|
||||
const bindValue = toBindValue(params);
|
||||
if (bindValue !== undefined) stmt.bind(bindValue);
|
||||
if (stmt.step()) return stmt.getAsObject();
|
||||
return undefined;
|
||||
} finally {
|
||||
@@ -127,7 +180,8 @@ export async function createSqlJsAdapter(filePath: string): Promise<SqliteAdapte
|
||||
all(...params: unknown[]): unknown[] {
|
||||
const stmt = db.prepare(sql);
|
||||
try {
|
||||
if (params.length) stmt.bind(params as unknown[]);
|
||||
const bindValue = toBindValue(params);
|
||||
if (bindValue !== undefined) stmt.bind(bindValue);
|
||||
const rows: unknown[] = [];
|
||||
while (stmt.step()) rows.push(stmt.getAsObject());
|
||||
return rows;
|
||||
|
||||
2
src/types/sqljs.d.ts
vendored
2
src/types/sqljs.d.ts
vendored
@@ -1,6 +1,6 @@
|
||||
declare module "sql.js" {
|
||||
export interface SqlJsStatement {
|
||||
bind(values: unknown[]): void;
|
||||
bind(values: unknown[] | Record<string, unknown>): void;
|
||||
step(): boolean;
|
||||
getAsObject(): Record<string, unknown>;
|
||||
free(): void;
|
||||
|
||||
@@ -85,6 +85,56 @@ describe("driverFactory", () => {
|
||||
adapter.close();
|
||||
});
|
||||
|
||||
// #6628 (remaining gap): concurrent preInitSqlJs() calls for the same
|
||||
// filePath must share ONE in-flight load instead of each caller
|
||||
// independently fs.readFileSync + WASM-decoding the whole file — the
|
||||
// thundering-herd amplifier of the OOM condition #6632 already partly
|
||||
// fixed (restore-cycle-breaker + OOM early-abort), left un-implemented by
|
||||
// the reporter's own proposed promise-sharing fix.
|
||||
test("preInitSqlJs shares one in-flight load across concurrent callers", async (t) => {
|
||||
const os = await import("node:os");
|
||||
const path = await import("node:path");
|
||||
// The dynamic-import namespace object is read-only; grab the mutable CJS
|
||||
// `.default` (== module.exports) so readFileSync can be monkeypatched.
|
||||
const fsNs = await import("node:fs");
|
||||
const fs = fsNs.default;
|
||||
|
||||
const tmpFile = path.join(os.tmpdir(), `sqljs_race_${Date.now()}.sqlite`);
|
||||
fs.writeFileSync(tmpFile, Buffer.alloc(1024 * 1024, 1));
|
||||
t.after(() => {
|
||||
try {
|
||||
fs.unlinkSync(tmpFile);
|
||||
} catch {}
|
||||
});
|
||||
|
||||
let readCountForTarget = 0;
|
||||
const originalReadFileSync = fs.readFileSync;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(fs as any).readFileSync = (...args: Parameters<typeof fs.readFileSync>) => {
|
||||
if (args[0] === tmpFile) readCountForTarget += 1;
|
||||
return originalReadFileSync(...args);
|
||||
};
|
||||
t.after(() => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(fs as any).readFileSync = originalReadFileSync;
|
||||
});
|
||||
|
||||
const [a, b, c] = await Promise.all([
|
||||
preInitSqlJs(tmpFile),
|
||||
preInitSqlJs(tmpFile),
|
||||
preInitSqlJs(tmpFile),
|
||||
]);
|
||||
|
||||
assert.equal(
|
||||
readCountForTarget,
|
||||
1,
|
||||
`expected exactly 1 shared full-file read for 3 concurrent preInitSqlJs() calls, got ${readCountForTarget}`
|
||||
);
|
||||
assert.equal(a, b, "concurrent callers must resolve to the SAME adapter instance");
|
||||
assert.equal(b, c, "concurrent callers must resolve to the SAME adapter instance");
|
||||
a.close();
|
||||
});
|
||||
|
||||
test("cross-driver: escreve com adapter sync, relê com sql.js", async (t) => {
|
||||
const os = await import("node:os");
|
||||
const path = await import("node:path");
|
||||
|
||||
@@ -89,4 +89,90 @@ describe("sqljsAdapter", () => {
|
||||
assert.equal(row.name, "test-value");
|
||||
reader.close();
|
||||
});
|
||||
|
||||
// #6802: named-parameter object binds ("... WHERE is_active = @isActive"
|
||||
// called as .all({ isActive: 1 })) must work the same way they do against
|
||||
// better-sqlite3, instead of throwing sql.js's own
|
||||
// "Wrong API use : tried to bind a value of an unknown type (...)." error.
|
||||
describe("named-parameter object bind (#6802)", () => {
|
||||
test("all() with a single named-params object mirrors getProviderConnections", async () => {
|
||||
const adapter = await createSqlJsAdapter(":memory:");
|
||||
adapter.exec(
|
||||
"CREATE TABLE provider_connections (id INTEGER PRIMARY KEY, provider TEXT, is_active INTEGER)"
|
||||
);
|
||||
adapter
|
||||
.prepare("INSERT INTO provider_connections (provider, is_active) VALUES (?, ?)")
|
||||
.run("glm", 1);
|
||||
adapter
|
||||
.prepare("INSERT INTO provider_connections (provider, is_active) VALUES (?, ?)")
|
||||
.run("openai", 0);
|
||||
|
||||
const sql =
|
||||
"SELECT * FROM provider_connections WHERE is_active = @isActive ORDER BY id ASC";
|
||||
const rows = adapter.prepare(sql).all({ isActive: 1 }) as Array<{ provider: string }>;
|
||||
|
||||
assert.equal(rows.length, 1, "expected exactly 1 active provider connection");
|
||||
assert.equal(rows[0].provider, "glm");
|
||||
adapter.close();
|
||||
});
|
||||
|
||||
test("get() with a single named-params object resolves the row", async () => {
|
||||
const adapter = await createSqlJsAdapter(":memory:");
|
||||
adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)");
|
||||
adapter.prepare("INSERT INTO t (val) VALUES (?)").run("named-get");
|
||||
|
||||
const row = adapter.prepare("SELECT val FROM t WHERE id = @id").get({ id: 1 }) as {
|
||||
val: string;
|
||||
};
|
||||
assert.equal(row.val, "named-get");
|
||||
adapter.close();
|
||||
});
|
||||
|
||||
test("run() with a single named-params object binds correctly", async () => {
|
||||
const adapter = await createSqlJsAdapter(":memory:");
|
||||
adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)");
|
||||
const result = adapter
|
||||
.prepare("INSERT INTO t (val) VALUES (@val)")
|
||||
.run({ val: "named-run" });
|
||||
assert.equal(result.changes, 1);
|
||||
|
||||
const row = adapter
|
||||
.prepare("SELECT val FROM t WHERE id = ?")
|
||||
.get(result.lastInsertRowid) as { val: string };
|
||||
assert.equal(row.val, "named-run");
|
||||
adapter.close();
|
||||
});
|
||||
|
||||
test("supports :name and $name sigils in addition to @name", async () => {
|
||||
const adapter = await createSqlJsAdapter(":memory:");
|
||||
adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)");
|
||||
adapter.prepare("INSERT INTO t (val) VALUES (?)").run("colon-sigil");
|
||||
adapter.prepare("INSERT INTO t (val) VALUES (?)").run("dollar-sigil");
|
||||
|
||||
const colonRow = adapter.prepare("SELECT val FROM t WHERE val = :val").get({
|
||||
val: "colon-sigil",
|
||||
}) as { val: string };
|
||||
assert.equal(colonRow.val, "colon-sigil");
|
||||
|
||||
const dollarRow = adapter.prepare("SELECT val FROM t WHERE val = $val").get({
|
||||
val: "dollar-sigil",
|
||||
}) as { val: string };
|
||||
assert.equal(dollarRow.val, "dollar-sigil");
|
||||
adapter.close();
|
||||
});
|
||||
|
||||
test("existing positional-array binding still works unchanged", async () => {
|
||||
const adapter = await createSqlJsAdapter(":memory:");
|
||||
adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT, b TEXT)");
|
||||
const result = adapter.prepare("INSERT INTO t (a, b) VALUES (?, ?)").run("x", "y");
|
||||
assert.equal(result.changes, 1);
|
||||
|
||||
const row = adapter
|
||||
.prepare("SELECT a, b FROM t WHERE id = ?")
|
||||
.get(result.lastInsertRowid) as { a: string; b: string };
|
||||
assert.equal(row.a, "x");
|
||||
assert.equal(row.b, "y");
|
||||
adapter.close();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user