mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-25 12:00:02 +08:00
fix(db): reopening an interrupted index no longer gets the server killed at every start (#2794)
Root cause: a run killed inside a bulk-load window (a crash, an OOM kill,
the liveness watchdog itself) leaves that window's secondary indexes
dropped, and since PR #1562 every writable open rebuilds them in
DatabaseConnection.open(), one CREATE INDEX after another on the caller's
thread. Each one scans and sorts its whole table. On a large index the sort
writes nothing to the database or its WAL, so on an MCP server the liveness
watchdog saw a silent main thread with no disk progress, and killed the
process before the index committed. Each restart died at the same index,
and the proxy's in-process fallback died the same way on the request it
re-served.
Fix: an open on a watched event loop leaves the rebuild to a worker thread
with its own connection, and the project is served once the rebuild is
done.
- DatabaseConnection.open({ deferIndexRecovery }) records the missing DDL
instead of running it; recoverIndexesOffThread() runs it on a worker
(single-flight, rejects on failure so the next call retries; SQLite's
default temp store, so the sort spills to disk; the worker collects its
garbage and exits by itself, per worker-teardown.ts).
- CodeGraph.open() (the MCP default project, the CLI) defers and awaits it.
- openSync({ deferIndexRecovery }) plus cg.recoverIndexes() for the MCP's
synchronous opens: the ToolHandler's projectPath projects and the engine's
retry path. ToolHandler.execute awaits the rebuild before serving (not
time-boxed: reads without the indexes scan whole tables), the project
lifecycle awaits it before its catch-up sync writes, and the retry path
starts the query pool after it.
- Synchronous opens that ask for nothing keep rebuilding in line, as before.
Validation:
- Reproduced on main b2a84873: a django 79502d0 init killed 1.5s into the
resolution window left the field report's index set missing (5 ref + 5
edge indexes, index_state indexing); the in-line rebuild is about 750ms
of main-thread work there.
- Real daemon on Windows, a 3M-ref / 1M-edge index left the same way
(781 MB), watchdog 1.5s: main was killed in 4 of 4 starts, stuck at 2 of
10 indexes, the launcher's in-process fallback killed too, no reply. The
fix rebuilt all 10 in about 11s with no main-thread block over 506ms,
answered the call, and the next start opened in about 120ms.
- New __tests__/index-recovery-off-thread.test.ts (9 cases): main's source
8 fail / 1 pass (the in-line guard), fix 9 pass. Spies show no CREATE
INDEX on the server thread for CodeGraph.open, a projectPath call
(watched and not), the retry path and the startup path; a real watchdog
(500ms) kills main mid-rebuild of a 300k-ref index and leaves the fix
alone. 6 mutations each failed at least one case.
- Contributor PR #1888 and the stale draft PR #1919 were read for guidance;
this ports their worker recovery to current main's open paths.
Fixes #1887
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b2a84873ed
commit
8073637d66
+106
-6
@@ -160,9 +160,16 @@ export class DatabaseConnection {
|
||||
}
|
||||
|
||||
/**
|
||||
* Open an existing database
|
||||
* Open an existing database.
|
||||
*
|
||||
* `deferIndexRecovery` leaves the secondary indexes an interrupted bulk load
|
||||
* dropped for {@link recoverIndexesOffThread} instead of rebuilding them
|
||||
* here. Rebuilding one scans and sorts its whole table: on a large index
|
||||
* that is minutes of synchronous work, and on a watched event loop the
|
||||
* liveness watchdog kills the process before the index commits, every time
|
||||
* it starts (#1887). Reads stay correct meanwhile, only slower.
|
||||
*/
|
||||
static open(dbPath: string, options: { readOnly?: boolean } = {}): DatabaseConnection {
|
||||
static open(dbPath: string, options: { readOnly?: boolean; deferIndexRecovery?: boolean } = {}): DatabaseConnection {
|
||||
if (!fs.existsSync(dbPath)) {
|
||||
throw new Error(`Database not found: ${dbPath}`);
|
||||
}
|
||||
@@ -194,7 +201,8 @@ export class DatabaseConnection {
|
||||
// beginBulkNodeLoad and endBulkNodeLoad): the FTS triggers are missing and
|
||||
// nodes_fts is stale. Rebuild + recreate so search stays in sync.
|
||||
conn.healBulkNodeLoad();
|
||||
conn.healBulkSecondaryIndexes();
|
||||
if (options.deferIndexRecovery) conn.deferredIndexDdls = conn.missingBulkSecondaryIndexDdls();
|
||||
else conn.healBulkSecondaryIndexes();
|
||||
|
||||
// Self-heal a killed session's leftover oversized WAL (#1431) — one
|
||||
// statSync when healthy, off-thread checkpoint+truncate when not.
|
||||
@@ -436,6 +444,14 @@ export class DatabaseConnection {
|
||||
|
||||
/** Recreate every secondary index a killed bulk parse/ref/edge window may leave dropped. */
|
||||
private healBulkSecondaryIndexes(): void {
|
||||
for (const ddl of this.missingBulkSecondaryIndexDdls()) this.db.exec(ddl);
|
||||
}
|
||||
|
||||
/**
|
||||
* The DDL that recreates every secondary index a killed bulk parse/ref/edge
|
||||
* window may leave dropped; empty, after one count, when none is missing.
|
||||
*/
|
||||
private missingBulkSecondaryIndexDdls(): string[] {
|
||||
const names = [...new Set<string>([
|
||||
...DatabaseConnection.BULK_PARSE_INDEX_NAMES,
|
||||
...DatabaseConnection.BULK_REF_INDEX_NAMES,
|
||||
@@ -445,15 +461,99 @@ export class DatabaseConnection {
|
||||
const row = this.db
|
||||
.prepare(`SELECT count(*) AS c FROM sqlite_master WHERE type = 'index' AND name IN (${placeholders})`)
|
||||
.get(...names) as { c: number } | undefined;
|
||||
if ((row?.c ?? 0) >= names.length) return;
|
||||
if ((row?.c ?? 0) >= names.length) return [];
|
||||
|
||||
const schemaPath = path.join(__dirname, 'schema.sql');
|
||||
const schema = fs.readFileSync(schemaPath, 'utf-8');
|
||||
for (const idx of names) {
|
||||
return names.map((idx) => {
|
||||
const m = schema.match(new RegExp(`CREATE INDEX IF NOT EXISTS ${idx}\\b[^;]*;`));
|
||||
if (!m) throw new Error(`schema.sql: index ${idx} not found for crash recovery`);
|
||||
this.db.exec(m[0]);
|
||||
return m[0];
|
||||
});
|
||||
}
|
||||
|
||||
/** Index DDL {@link open} left for {@link recoverIndexesOffThread}. */
|
||||
private deferredIndexDdls: string[] = [];
|
||||
private indexRecovery: Promise<void> | null = null;
|
||||
|
||||
/** Whether {@link open} left dropped secondary indexes for {@link recoverIndexesOffThread}. */
|
||||
hasDeferredIndexRecovery(): boolean {
|
||||
return this.deferredIndexDdls.length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recreate the secondary indexes {@link open} deferred, on a worker thread
|
||||
* with its own connection, so this thread's event loop, and the #850
|
||||
* liveness watchdog's heartbeat, keep turning however long the rebuild takes
|
||||
* (#1887). Each index commits on its own, so an interrupted recovery keeps
|
||||
* what it finished. Not best-effort like {@link runMaintenance}: rejects when
|
||||
* a statement fails, and the next call tries again. Single-flight; resolves
|
||||
* at once when nothing was deferred.
|
||||
*/
|
||||
recoverIndexesOffThread(): Promise<void> {
|
||||
if (this.deferredIndexDdls.length === 0) return Promise.resolve();
|
||||
this.indexRecovery ??= this.runRecoveryOffThread(this.deferredIndexDdls)
|
||||
.then(() => { this.deferredIndexDdls = []; })
|
||||
.finally(() => { this.indexRecovery = null; });
|
||||
return this.indexRecovery;
|
||||
}
|
||||
|
||||
private async runRecoveryOffThread(statements: string[]): Promise<void> {
|
||||
let WorkerCtor: typeof import('node:worker_threads').Worker;
|
||||
try {
|
||||
WorkerCtor = (await import('node:worker_threads')).Worker;
|
||||
} catch {
|
||||
// No worker threads in this runtime: recover here, as open() would have.
|
||||
for (const ddl of statements) this.db.exec(ddl);
|
||||
return;
|
||||
}
|
||||
// SQLite's default temp storage, not this connection's MEMORY: the sort
|
||||
// behind a big index spills to disk instead of growing the heap. The
|
||||
// worker collects its garbage before it ends, so it exits with no marking
|
||||
// in flight (worker-teardown.ts), and nothing terminates it.
|
||||
const workerSource = `
|
||||
const { workerData, parentPort } = require('node:worker_threads');
|
||||
let error = null;
|
||||
try {
|
||||
const { DatabaseSync } = require('node:sqlite');
|
||||
const db = new DatabaseSync(workerData.dbPath);
|
||||
try {
|
||||
db.exec('PRAGMA busy_timeout = 30000');
|
||||
for (const sql of workerData.statements) db.exec(sql);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
} catch (e) {
|
||||
error = String((e && e.message) || e);
|
||||
}
|
||||
try {
|
||||
const v8 = require('v8');
|
||||
v8.setFlagsFromString('--expose-gc');
|
||||
const gc = require('vm').runInNewContext('gc');
|
||||
v8.setFlagsFromString('--no-expose-gc');
|
||||
gc();
|
||||
} catch {}
|
||||
parentPort.postMessage({ error });
|
||||
`;
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
let settled = false;
|
||||
const worker = new WorkerCtor(workerSource, { eval: true, workerData: { dbPath: this.dbPath, statements } });
|
||||
worker.once('message', (message: { error: string | null }) => {
|
||||
settled = true;
|
||||
if (message.error) reject(new Error(`Could not restore the indexes of ${this.dbPath}: ${message.error}`));
|
||||
else resolve();
|
||||
});
|
||||
worker.once('error', (err) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
reject(err);
|
||||
});
|
||||
worker.once('exit', (code) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
reject(new Error(`Restoring the indexes of ${this.dbPath} stopped early (worker exit ${code}).`));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+36
-5
@@ -393,9 +393,19 @@ export class CodeGraph {
|
||||
throw new Error(`Invalid CodeGraph directory: ${validation.errors.join(', ')}`);
|
||||
}
|
||||
|
||||
// Open database
|
||||
// Open database. The secondary indexes an interrupted bulk load dropped are
|
||||
// rebuilt off this thread, and finished before the instance is returned
|
||||
// (#1887): rebuilt in line on a large index, they blocked the thread longer
|
||||
// than the liveness watchdog allows, and every restart was killed the same
|
||||
// way.
|
||||
const dbPath = getDatabasePath(resolvedRoot);
|
||||
const db = DatabaseConnection.open(dbPath, { readOnly: options.readOnly });
|
||||
const db = DatabaseConnection.open(dbPath, { readOnly: options.readOnly, deferIndexRecovery: true });
|
||||
try {
|
||||
await db.recoverIndexesOffThread();
|
||||
} catch (err) {
|
||||
db.close();
|
||||
throw err;
|
||||
}
|
||||
const queries = new QueryBuilder(db.getDb());
|
||||
|
||||
const instance = new CodeGraph(db, queries, resolvedRoot);
|
||||
@@ -462,9 +472,14 @@ export class CodeGraph {
|
||||
}
|
||||
|
||||
/**
|
||||
* Open synchronously (without sync)
|
||||
* Open synchronously (without sync).
|
||||
*
|
||||
* `deferIndexRecovery` leaves the secondary indexes an interrupted bulk load
|
||||
* dropped for {@link recoverIndexes} to rebuild off this thread, instead of
|
||||
* rebuilding them here (#1887): for a caller on a watched event loop, which
|
||||
* then awaits {@link recoverIndexes} before it relies on the project.
|
||||
*/
|
||||
static openSync(projectRoot: string, options: Pick<OpenOptions, 'readOnly'> = {}): CodeGraph {
|
||||
static openSync(projectRoot: string, options: Pick<OpenOptions, 'readOnly'> & { deferIndexRecovery?: boolean } = {}): CodeGraph {
|
||||
const resolvedRoot = path.resolve(projectRoot);
|
||||
|
||||
// Check if initialized
|
||||
@@ -480,12 +495,28 @@ export class CodeGraph {
|
||||
|
||||
// Open database
|
||||
const dbPath = getDatabasePath(resolvedRoot);
|
||||
const db = DatabaseConnection.open(dbPath, { readOnly: options.readOnly });
|
||||
const db = DatabaseConnection.open(dbPath, { readOnly: options.readOnly, deferIndexRecovery: options.deferIndexRecovery });
|
||||
const queries = new QueryBuilder(db.getDb());
|
||||
|
||||
return new CodeGraph(db, queries, resolvedRoot);
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuild, on a worker thread, the secondary indexes an interrupted bulk
|
||||
* load dropped and `openSync({ deferIndexRecovery: true })` left (#1887).
|
||||
* Resolves at once when nothing is left, which is every open but the first
|
||||
* after such an interruption; rejects when the rebuild fails, and the next
|
||||
* call tries again. Reads stay correct meanwhile, only slower.
|
||||
*/
|
||||
recoverIndexes(): Promise<void> {
|
||||
return this.db.recoverIndexesOffThread();
|
||||
}
|
||||
|
||||
/** Whether {@link recoverIndexes} still has indexes to rebuild. */
|
||||
hasPendingIndexRecovery(): boolean {
|
||||
return this.db.hasDeferredIndexRecovery();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a directory has been initialized as a CodeGraph project
|
||||
*/
|
||||
|
||||
+13
-2
@@ -246,12 +246,23 @@ export class MCPEngine {
|
||||
this.cg = null;
|
||||
}
|
||||
assertNoRebuild(resolvedRoot);
|
||||
this.cg = loadCodeGraph().openSync(resolvedRoot, { readOnly: this.opts.readOnly });
|
||||
// Indexes an interrupted bulk load dropped are rebuilt off this thread
|
||||
// (#1887); the catch-up and tool calls wait for them.
|
||||
this.cg = loadCodeGraph().openSync(resolvedRoot, { readOnly: this.opts.readOnly, deferIndexRecovery: true });
|
||||
this.projectPath = resolvedRoot;
|
||||
this.toolHandler.setDefaultCodeGraph(this.cg);
|
||||
this.startWatching();
|
||||
this.catchUpSync();
|
||||
this.maybeStartPool(resolvedRoot);
|
||||
// The pool's workers open the project themselves: start them on a
|
||||
// rebuilt one, not racing the rebuild for its write lock.
|
||||
const opened = this.cg;
|
||||
if (opened?.hasPendingIndexRecovery()) {
|
||||
void opened.recoverIndexes().catch(() => { /* reported by the tool call waiting on it */ }).then(() => {
|
||||
if (this.cg === opened) this.maybeStartPool(resolvedRoot);
|
||||
});
|
||||
} else {
|
||||
this.maybeStartPool(resolvedRoot);
|
||||
}
|
||||
} catch (err) {
|
||||
// Still failing — caller will try again on the next tool call.
|
||||
this.toolHandler.setDefaultOpenFailure(err instanceof WslSharedIndexError ? err : null);
|
||||
|
||||
@@ -118,6 +118,10 @@ function ready(root: string, project: Project): Promise<void> {
|
||||
|
||||
async function activate(root: string, project: Project): Promise<void> {
|
||||
if (!isInitialized(root)) return;
|
||||
// Rebuild what an interrupted bulk load dropped (off this thread, #1887)
|
||||
// before writing: a sync's writes would wait out the rebuild's write lock
|
||||
// with this thread blocked.
|
||||
if (project.cg.hasPendingIndexRecovery?.()) await project.cg.recoverIndexes();
|
||||
const writer = tryAcquireWriterLock(root, 'fallback');
|
||||
if (writer.kind === 'taken') {
|
||||
// Keep a real daemon session, so its idle timeout cannot strand this reader.
|
||||
|
||||
+23
-1
@@ -2425,7 +2425,9 @@ export class ToolHandler {
|
||||
}
|
||||
}
|
||||
|
||||
const open = () => loadCodeGraph().openSync(canonicalRoot);
|
||||
// Indexes an interrupted bulk load dropped are rebuilt off this thread,
|
||||
// before the project is served (awaitIndexRecovery, #1887).
|
||||
const open = () => loadCodeGraph().openSync(canonicalRoot, { deferIndexRecovery: true });
|
||||
const cg = this.projectLifecycle?.open(canonicalRoot, open) ?? open();
|
||||
this.projectCache.set(canonicalRoot, cg);
|
||||
this.projectUsedAt.set(canonicalRoot, Date.now());
|
||||
@@ -2461,6 +2463,25 @@ export class ToolHandler {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait for a project to finish rebuilding, off this thread, the secondary
|
||||
* indexes an interrupted bulk load dropped (#1887). Not time-boxed like the
|
||||
* catch-up gate: without them a read scans whole tables, which on a large
|
||||
* index stalls the thread the rebuild was moved off. Never throws: a failed
|
||||
* rebuild leaves reads correct, and the next call tries again.
|
||||
*/
|
||||
private async awaitIndexRecovery(cg: CodeGraph): Promise<void> {
|
||||
if (!cg.hasPendingIndexRecovery?.()) return;
|
||||
try {
|
||||
await cg.recoverIndexes();
|
||||
} catch (err) {
|
||||
process.stderr.write(
|
||||
`[CodeGraph MCP] Could not restore the indexes an interrupted indexing run left out of ${cg.getProjectRoot()}: ` +
|
||||
`${err instanceof Error ? err.message : String(err)}. Queries still work, more slowly; the next call tries again.\n`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async awaitProjectGate(projectPath: string): Promise<void> {
|
||||
const cg = this.getCodeGraph(projectPath);
|
||||
if (!this.projectLifecycle || cg === this.cg) return;
|
||||
@@ -2906,6 +2927,7 @@ export class ToolHandler {
|
||||
}
|
||||
|
||||
const project = await this.getCodeGraph(args.projectPath as string | undefined);
|
||||
await this.awaitIndexRecovery(project);
|
||||
// Recover a watcher disabled by prolonged lock contention on the next call.
|
||||
// The stale banner remains until the watcher finishes its full scan;
|
||||
// frequent calls cannot bypass its cooldown (#1959).
|
||||
|
||||
Reference in New Issue
Block a user