Просмотр исходного кода

fix(db): fail closed when WAL valve cannot checkpoint past caps (#1539) (#1751)

sync() armed the WAL valve but never called backpressure(), so daemon
catch-up could grow the WAL without bound while query-pool readers pinned
frames. Wire the writer pause into sync store + batched resolution, and
abort with WalValveAbortError after parked backfills fail past the
documented hard/file caps instead of disabling parking for 60s.

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 7 часов назад
Родитель
Сommit
9b8bb4aba0
5 измененных файлов с 199 добавлено и 38 удалено
  1. 2 0
      CHANGELOG.md
  2. 104 15
      __tests__/wal-deferral.test.ts
  3. 72 19
      src/db/wal-valve.ts
  4. 10 1
      src/extraction/index.ts
  5. 11 3
      src/index.ts

+ 2 - 0
CHANGELOG.md

@@ -137,6 +137,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 #### MCP / indexing
 
+- **Live sync no longer lets the write-ahead log grow without a bound when a reader is holding it open (#1539).** Incremental sync now uses the same writer pause that full indexing already used, and if checkpointing still cannot finish once the log is past its documented size limit — typically because the query pool is reading at the same time — sync stops with a clear error instead of keeping writing until the disk fills. The previous behaviour could leave a multi-tens-of-gigabyte log beside a few-gigabyte index on a large project. Close concurrent readers and retry, or raise `CODEGRAPH_WAL_VALVE_MB` if the limit is too tight for the project.
+
 - **A second `codegraph serve --mcp` on the same project no longer silently kills auto-sync (#1740).** Direct mode (`CODEGRAPH_NO_DAEMON=1` or proxy→in-process fallback) now takes an exclusive `.codegraph/writer.pid` lock; a second writer exits immediately with guidance to stop the other server or unset `CODEGRAPH_NO_DAEMON` so clients share the daemon. The shared daemon already multiplexes N clients onto one watcher — this closes the same-OS dual-direct gap the docs warned about for Windows/WSL but did not guard.
 
 - Indexing no longer checks whether files outside your project exist. A relative import that points above the project directory (`../../something`) made CodeGraph probe that location on disk while resolving it. Nothing outside the project was ever read, and no such file was ever added to the index or linked to, but the check itself should not have happened — such an import now simply resolves to nothing. Symlinks inside your project that point at code kept elsewhere are unaffected and still index as before. Thanks @ErQrYfkrju. (#1631)

+ 104 - 15
__tests__/wal-deferral.test.ts

@@ -9,12 +9,12 @@
  * the valve's trigger/dedupe/backpressure logic, and the end-to-end indexAll
  * behavior (identical graph with and without deferral; interval restored).
  */
-import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import { DatabaseConnection } from '../src/db';
-import { WalCheckpointValve, resolveWalValveMb } from '../src/db/wal-valve';
+import { WalCheckpointValve, WalValveAbortError, resolveWalValveMb } from '../src/db/wal-valve';
 import CodeGraph from '../src/index';
 
 let tmpDir: string;
@@ -193,6 +193,20 @@ function writeFixtureProject(): void {
   }
 }
 
+
+async function seedPendingRefs(cg: CodeGraph): Promise<void> {
+  const raw = (cg as unknown as { db: DatabaseConnection }).db.getDb();
+  const node = raw.prepare("SELECT id, file_path FROM nodes WHERE kind = 'function' LIMIT 1").get() as
+    | { id: string; file_path: string }
+    | undefined;
+  expect(node).toBeDefined();
+  const ins = raw.prepare(
+    "INSERT INTO unresolved_refs (from_node_id, reference_name, reference_kind, line, col, file_path, language, status) VALUES (?, ?, 'calls', 1, 0, ?, 'typescript', 'pending')"
+  );
+  ins.run(node!.id, 'helper0', node!.file_path);
+  ins.run(node!.id, 'helper1', node!.file_path);
+}
+
 describe('indexAll WAL deferral end-to-end', () => {
 
   it('produces the same graph with and without deferral, and restores the interval', async () => {
@@ -298,6 +312,35 @@ describe('sync WAL deferral end-to-end (#1248)', () => {
       delete process.env.CODEGRAPH_NO_WAL_DEFER;
     }
   });
+
+  it('applies WAL backpressure during changed-file storage and orphan resolution (#1539)', async () => {
+    writeFixtureProject();
+    const cg = CodeGraph.initSync(tmpDir);
+    await cg.indexAll();
+    const backpressure = vi
+      .spyOn(WalCheckpointValve.prototype, 'backpressure')
+      .mockReturnValue(null);
+
+    try {
+      fs.writeFileSync(
+        path.join(tmpDir, 'src', 'mod0.ts'),
+        `export function fn0(x: number): number { return helper0(x) + 100; }\n` +
+        `function helper0(x: number): number { return x * 100; }\n`
+      );
+      const changed = await cg.sync();
+      expect(changed.filesModified).toBe(1);
+      expect(backpressure).toHaveBeenCalled();
+
+      backpressure.mockClear();
+      await seedPendingRefs(cg);
+      const recovered = await cg.sync();
+      expect(recovered.filesAdded + recovered.filesModified + recovered.filesRemoved).toBe(0);
+      expect(backpressure).toHaveBeenCalled();
+    } finally {
+      backpressure.mockRestore();
+      await cg.close();
+    }
+  });
 });
 
 describe('resolution-phase WAL backpressure plumbing (§7a.1)', () => {
@@ -308,19 +351,6 @@ describe('resolution-phase WAL backpressure plumbing (§7a.1)', () => {
   // 22GB WAL on a 4.6GB DB. These pin that the batch loop (a) calls the hook
   // at the pool-idle boundary and (b) actually parks on a returned promise.
 
-  async function seedPendingRefs(cg: CodeGraph): Promise<void> {
-    const raw = (cg as unknown as { db: DatabaseConnection }).db.getDb();
-    const node = raw.prepare("SELECT id, file_path FROM nodes WHERE kind = 'function' LIMIT 1").get() as
-      | { id: string; file_path: string }
-      | undefined;
-    expect(node).toBeDefined();
-    const ins = raw.prepare(
-      "INSERT INTO unresolved_refs (from_node_id, reference_name, reference_kind, line, col, file_path, language, status) VALUES (?, ?, 'calls', 1, 0, ?, 'typescript', 'pending')"
-    );
-    ins.run(node!.id, 'helper0', node!.file_path);
-    ins.run(node!.id, 'helper1', node!.file_path);
-  }
-
   it('calls the backpressure hook once per settled batch', async () => {
     writeFixtureProject();
     const cg = CodeGraph.initSync(tmpDir);
@@ -417,6 +447,65 @@ describe('valve file-size trigger (§7a.1: backfilled WAL still grows the file)'
   });
 });
 
+
+describe('WAL valve fail-closed (#1539)', () => {
+  it('aborts with WalValveAbortError when parked backfills cannot progress past the file cap', async () => {
+    const db = openDb();
+    db.setWalAutocheckpoint(0);
+    writeRows(db, 800); // well past a 0.5MB soft / 2MB file cap
+    expect(db.getWalSizeBytes()).toBeGreaterThan(2 * 1024 * 1024);
+
+    const valve = new WalCheckpointValve(db, 0.5);
+    // Simulate a reader pinning every PASSIVE/TRUNCATE attempt.
+    db.checkpointWalPassive = async () => ({ busy: 1, log: 100, checkpointed: 0 });
+    db.checkpointWalTruncate = async () => ({ busy: 1, log: 100, checkpointed: 0 });
+
+    const bp = valve.backpressure();
+    expect(bp).not.toBeNull();
+    await expect(bp!).rejects.toBeInstanceOf(WalValveAbortError);
+    try {
+      await bp!;
+    } catch (err) {
+      expect(err).toMatchObject({
+        name: 'WalValveAbortError',
+        code: 'WAL_VALVE_ABORT',
+      });
+      expect((err as WalValveAbortError).message).toMatch(/Aborting to avoid unbounded disk growth/);
+      expect((err as WalValveAbortError).walBytes).toBeGreaterThan((err as WalValveAbortError).fileCapBytes);
+    }
+    // Caps remain enforceable: a subsequent backpressure call still parks (no
+    // futility latch that returns null and lets the writer race past the cap).
+    const again = valve.backpressure();
+    expect(again).not.toBeNull();
+    await expect(again!).rejects.toBeInstanceOf(WalValveAbortError);
+    db.close();
+  });
+
+  it('aborts when checkpoint machinery is unavailable while over the file cap', async () => {
+    const db = openDb();
+    db.setWalAutocheckpoint(0);
+    writeRows(db, 800);
+    const valve = new WalCheckpointValve(db, 0.5);
+    db.checkpointWalPassive = async () => null;
+    const bp = valve.backpressure();
+    expect(bp).not.toBeNull();
+    await expect(bp!).rejects.toBeInstanceOf(WalValveAbortError);
+    db.close();
+  });
+
+  it('does not abort a soft foldNow give-up that stays under both caps', async () => {
+    const db = openDb();
+    db.setWalAutocheckpoint(0);
+    writeRows(db, 50); // small WAL
+    const valve = new WalCheckpointValve(db, 1024); // 1GB soft — hard 2GB, fileCap 4GB
+    db.checkpointWalPassive = async () => ({ busy: 1, log: 10, checkpointed: 0 });
+    // foldNow calls backfillFully even with modest growth; under caps this is soft.
+    await expect(valve.foldNow()).resolves.toBeUndefined();
+    expect(valve.backpressure()).toBeNull();
+    db.close();
+  });
+});
+
 describe('resolveWalValveMb DB-size scaling (§7a.2 fold-tax reduction)', () => {
   it('scales soft cap ~dbSize/4 within [256, 2048]MB; env always wins', () => {
     const GB = 1024 * 1024 * 1024;

+ 72 - 19
src/db/wal-valve.ts

@@ -42,10 +42,39 @@
  * the next pass covers everything, the WAL wraps on the following commit,
  * and the pause is the disk's honest catch-up cost — the correct terminal
  * mode when hardware genuinely can't keep up with the append rate.
+ *
+ * Fail-closed (#1539): if parked backfills cannot progress (a reader pinning
+ * frames) while the WAL is past the hard/file caps, the valve throws
+ * {@link WalValveAbortError} instead of releasing the writer. The previous
+ * "futility latch" disabled parking for 60s after consecutive give-ups so a
+ * pinned reader would not churn checkpoint workers — but that also let the
+ * WAL grow without a bound (observed 64 GiB on a kernel-scale daemon catch-up
+ * with the query pool holding read marks). Aborting with a clear error is the
+ * safe terminal mode; the caller closes readers / retries once the pin clears.
  */
 
 import type { DatabaseConnection } from './index';
 
+/**
+ * Thrown when the valve cannot checkpoint past its documented caps while a
+ * reader pins WAL frames (#1539). Callers (index/sync) should surface this and
+ * stop writing rather than risk unbounded disk growth.
+ */
+export class WalValveAbortError extends Error {
+  readonly code = 'WAL_VALVE_ABORT' as const;
+  readonly walBytes: number;
+  readonly fileCapBytes: number;
+  readonly hardBytes: number;
+
+  constructor(message: string, sizes: { walBytes: number; fileCapBytes: number; hardBytes: number }) {
+    super(message);
+    this.name = 'WalValveAbortError';
+    this.walBytes = sizes.walBytes;
+    this.fileCapBytes = sizes.fileCapBytes;
+    this.hardBytes = sizes.hardBytes;
+  }
+}
+
 /** Soft WAL-growth threshold (MB) that triggers an off-thread passive checkpoint. */
 const DEFAULT_WAL_VALVE_MB = 256;
 /** Hard cap = this × soft threshold; past it the writer pauses for a full backfill. */
@@ -93,17 +122,10 @@ export class WalCheckpointValve {
   private readonly fileCapBytes: number;
 
   /**
-   * Futility latch: consecutive backfill give-ups (a reader pinning the WAL)
-   * disable further writer pauses for a cooldown, so a pinned phase degrades
-   * to the pre-valve behavior (unbounded WAL, folded when the pinner exits)
-   * instead of burning a 20-pass checkpoint attempt — each pass a worker
-   * thread + fresh connection — at EVERY over-cap boundary. That churn is
-   * what turned a pinned kernel-scale resolution from slow into OOM-killed
-   * (§7a.1 run 1: 22GB WAL, exit 137 at an envelope the pre-fix build
-   * survived).
+   * Consecutive parked-backfill give-ups. Used only for diagnostics in the
+   * abort message — parking is never disabled (#1539 fail-closed).
    */
   private consecutiveGiveUps = 0;
-  private futileUntil = 0;
 
   constructor(
     private readonly db: DatabaseConnection,
@@ -173,7 +195,6 @@ export class WalCheckpointValve {
    */
   backpressure(): Promise<void> | null {
     if (this.pause) return this.pause;
-    if (Date.now() < this.futileUntil) return null; // pinned reader — parking is churn, not progress
     // Two independent triggers:
     //  - growth: un-backfilled BACKLOG past the hard cap (the original valve).
     //  - file size: a WAL can stay fully backfilled and still grow without
@@ -219,15 +240,31 @@ export class WalCheckpointValve {
   /**
    * With the writer parked on the returned promise, loop passive passes until
    * one reports the entire WAL backfilled (typically the second: the first
-   * drains the pass that was already running against a stale snapshot). Gives
-   * up after a bounded number of passes — e.g. a reader pinning the WAL —
-   * because unbounded WAL growth degrades; a wedged writer never recovers.
+   * drains the pass that was already running against a stale snapshot). After
+   * a bounded number of passes without a full backfill — e.g. a reader
+   * pinning the WAL — throws {@link WalValveAbortError} when still past the
+   * hard/file caps (#1539 fail-closed). Soft give-up under those caps is
+   * reserved for foldNow on a modest backlog that could not complete.
    */
   private async backfillFully(): Promise<void> {
     for (let i = 0; i < MAX_PAUSED_BACKFILL_PASSES; i++) {
       if (this.inflight) await this.inflight; // fold in the stale in-flight pass first
       const res = await this.db.checkpointWalPassive();
-      if (!res) return; // checkpoint machinery unavailable — don't spin
+      if (!res) {
+        // Machinery unavailable: fail closed past the documented caps (#1539),
+        // otherwise soft-return so a non-WAL / closing connection does not abort.
+        const walBytes = this.db.getWalSizeBytes();
+        const growth = this.growthBytes();
+        if (walBytes > this.fileCapBytes || growth > this.hardBytes) {
+          throw new WalValveAbortError(
+            `WAL checkpoint machinery unavailable while over the documented cap ` +
+              `(wal=${this.mb(walBytes)}, fileCap=${this.mb(this.fileCapBytes)}). ` +
+              `Aborting to avoid unbounded disk growth.`,
+            { walBytes, fileCapBytes: this.fileCapBytes, hardBytes: this.hardBytes }
+          );
+        }
+        return;
+      }
       this.log(`backfill pass ${i + 1}: busy=${res.busy} log=${res.log} checkpointed=${res.checkpointed} wal=${this.mb(this.db.getWalSizeBytes())}`);
       if (res.busy === 0 && res.log === res.checkpointed) {
         // Backfill complete AND we are at a parked barrier (backfillFully only
@@ -240,21 +277,37 @@ export class WalCheckpointValve {
         if (trunc) this.log(`truncate: busy=${trunc.busy} wal=${this.mb(this.db.getWalSizeBytes())}`);
         this.sizeAtLastFullBackfill = this.db.getWalSizeBytes();
         this.consecutiveGiveUps = 0;
-        this.futileUntil = 0;
         return;
       }
     }
     this.consecutiveGiveUps++;
-    if (this.consecutiveGiveUps >= 2) {
-      this.futileUntil = Date.now() + 60_000;
-    }
-    const msg = `backfill gave up after ${MAX_PAUSED_BACKFILL_PASSES} passes (streak ${this.consecutiveGiveUps}${this.futileUntil ? ', parking disabled 60s' : ''}) — a reader is pinning the WAL`;
+    const walBytes = this.db.getWalSizeBytes();
+    const growth = this.growthBytes();
+    const msg =
+      `backfill gave up after ${MAX_PAUSED_BACKFILL_PASSES} passes ` +
+      `(streak ${this.consecutiveGiveUps}) — a reader is pinning the WAL ` +
+      `(wal=${this.mb(walBytes)} growth=${this.mb(growth)} ` +
+      `hard=${this.mb(this.hardBytes)} fileCap=${this.mb(this.fileCapBytes)})`;
     this.log(msg);
     // Give-ups are rare and load-bearing for §7a.1-class diagnosis — surface
     // them on any timing-instrumented run, not just valve-debug ones.
     if (process.env.CODEGRAPH_SYNTH_TIMINGS && !process.env.CODEGRAPH_WAL_VALVE_DEBUG) {
       console.error(`[wal-valve] ${msg}`);
     }
+    // Fail closed (#1539): never release the writer past the documented caps
+    // when checkpoints cannot progress. The old futility latch disabled
+    // parking for 60s and allowed unbounded growth (64 GiB observed).
+    if (walBytes > this.fileCapBytes || growth > this.hardBytes) {
+      throw new WalValveAbortError(
+        `WAL checkpoint cannot progress while a reader pins frames ` +
+          `(wal=${this.mb(walBytes)}, growth=${this.mb(growth)}, ` +
+          `fileCap=${this.mb(this.fileCapBytes)}, hard=${this.mb(this.hardBytes)}, ` +
+          `give-ups=${this.consecutiveGiveUps}). Aborting to avoid unbounded disk growth. ` +
+          `Close concurrent readers (for example the MCP query pool) and retry, ` +
+          `or raise CODEGRAPH_WAL_VALVE_MB if the threshold is too tight for this project.`,
+        { walBytes, fileCapBytes: this.fileCapBytes, hardBytes: this.hardBytes }
+      );
+    }
   }
 
   private fire(): void {

+ 10 - 1
src/extraction/index.ts

@@ -2712,7 +2712,13 @@ export class ExtractionOrchestrator {
      * set is not exactly known (directory removals, event overflow): the full
      * scan-diff remains the ground truth those cases need (#1285).
      */
-    scopedPaths?: string[]
+    scopedPaths?: string[],
+    /**
+     * Writer-side WAL pressure valve (#1539). Called after every changed file
+     * is stored, when no extraction transaction is open, so a checkpoint can
+     * safely catch up before the next file grows the WAL further.
+     */
+    backpressure?: () => Promise<void> | null
   ): Promise<SyncResult> {
     await initGrammars(); // Initialize WASM runtime (grammars loaded lazily below)
     const startTime = Date.now();
@@ -2912,6 +2918,9 @@ export class ExtractionOrchestrator {
 
       const result = await this.indexFile(filePath);
       nodesUpdated += result.nodes.length;
+
+      const pause = backpressure?.();
+      if (pause) await pause;
     }
 
     // Names whose definition set this sync changed: a `file\0name` pair present

+ 11 - 3
src/index.ts

@@ -816,7 +816,13 @@ export class CodeGraph {
           try { return this.queries.isNameSegmentVocabEmpty(); } catch { return false; }
         })();
 
-        const result = await this.orchestrator.sync(options.onProgress, options.paths);
+        // Writer-side backstop for deferred WAL checkpointing (#1539): sync
+        // previously armed the valve but never called backpressure(), so the
+        // hard/file caps were never enforced during daemon catch-up — only
+        // timer-driven PASSIVE checkpoints ran, and a query-pool reader could
+        // pin frames while the WAL grew without a bound.
+        const backpressure = walValve ? () => walValve!.backpressure() : undefined;
+        const result = await this.orchestrator.sync(options.onProgress, options.paths, backpressure);
 
         // Fold the store phase's WAL BEFORE the post-store reads below
         // (resolution reads on the main thread) — same rationale as
@@ -914,7 +920,8 @@ export class CodeGraph {
                   current: done,
                   total: totalPasses,
                 });
-              }
+              },
+              backpressure
             );
           }
         }
@@ -980,7 +987,8 @@ export class CodeGraph {
                 current: done,
                 total: totalPasses,
               });
-            }
+            },
+            backpressure
           );
         }