mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
* feat: parallel sync — bounded concurrent imports (#489) gbrain sync --concurrency N (alias --workers N) parallelizes the import phase using per-worker Postgres engine instances with an atomic queue index (same proven pattern as gbrain import --workers N). Auto-concurrency: when a sync touches >100 files and the user didn't explicitly set --concurrency, defaults to 4 workers. Small incremental syncs (<50 files) stay serial. Full syncs auto-detect Postgres and default to 4 workers. Minion sync handler defaults to concurrency=4, configurable via job params: {"concurrency": 8}. Delete and rename phases remain serial (order-dependent, fast). PGLite falls back to serial automatically (single-connection engine). Changes: - src/commands/sync.ts: SyncOpts.concurrency, parallel import loop in performSync incremental path, --workers passthrough in performFullSync - src/commands/jobs.ts: sync handler accepts concurrency param (default 4) - CHANGELOG.md: v0.23.0 parallel sync entry All 37 existing sync tests pass. Typecheck clean. * feat: shared concurrency policy + db-lock primitive src/core/sync-concurrency.ts — single source of truth for autoConcurrency() + parseWorkers() + shouldRunParallel() + constants. Replaces three drifted call-site policies (performSync, performFullSync, jobs handler). src/core/db-lock.ts — generic tryAcquireDbLock(engine, lockId, ttlMinutes) over the existing gbrain_cycle_locks table. Parameterized lock id so performSync (gbrain-sync) can nest cleanly under cycle.ts (gbrain-cycle) without deadlock. test/sync-concurrency.test.ts — 17 cases covering PGLite-forces-serial, explicit override clamping, auto-path threshold, parseWorkers validation (rejects 0, negatives, NaN, decimals, trailing chars). No consumers yet; subsequent commits wire sync.ts, import.ts, and jobs.ts to use these helpers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: harden performSync — writer lock, head-drift gate, engine.kind CODEX-2: wrap performSync body in a gbrain-sync DB lock so two concurrent syncs (manual + autopilot, two terminals, two Conductor workspaces) cannot both read last_commit, both write it unconditionally, and let the last writer win. cycle.ts continues to hold gbrain-cycle for its broader scope; the two ids nest cleanly. CODEX-3: capture git HEAD at sync entry, re-rev-parse after the import phase, refuse to advance last_commit if HEAD drifted (someone ran git checkout / git pull mid-sync). Vanished files now go into failedFiles instead of silent-skip — same gating mechanism, no more bookmark advance past unimported work. A1: replace both PGLite detection sites with engine.kind === 'pglite'. The constructor.name sniff is gone (breaks under bundling) and so is the inconsistent config?.engine string check. A2: connect worker engines serially into an array, run inside try/finally so disconnect always fires — even on partial connect failure, OOM, or mid-import abort. Prior Promise.all(...disconnect) leaked the 8 worker connections on any panic path. Q1: explicit --workers / opts.concurrency now bypasses the >50-file floor. User opt-in beats the auto-path safety net. Q3: drop the config!.database_url! non-null assertions; fall back to serial when database_url is unset instead of crashing on TypeError. Q4: worker-count banner moves from console.log to console.error so stdout stays clean for --json output. test/sync-parallel.test.ts — 7 cases over PGLite covering the bookmark gate under concurrency request, the head-drift gate, vanished-file failure capture, PGLite-stays-serial, and the writer-lock contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: import.ts — engine.kind discriminator, worker try/finally, parseWorkers A1: replace the config?.engine === 'pglite' string sniff with engine.kind === 'pglite' to match sync.ts and the v0.13.1 contract. A2: wrap worker engine creation + the parallel loop in try/finally so disconnects always fire — same pattern as sync.ts. Worker engines now push onto an array as they connect (rather than Promise.all) so the finally block can clean up partial-connect state. Q2: route --workers parsing through the shared parseWorkers() helper. parseInt-with-no-validation is gone — '0', '-3', 'foo', '1.5' now exit with a clear error message instead of silently falling through. Q3: drop the config!.database_url! non-null assertion; fall back to serial when database_url is unset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: jobs.ts sync handler — resolve sourceId, autoConcurrency CODEX-1: resolve sourceId at handler entry by looking up sources.local_path. Mirrors cycle.ts:480's autopilot-cycle fix (PR #475). Without this, every Minion sync job on a multi-source brain reads global config.sync.last_commit instead of the per-source anchor, which on a regularly-GC'd repo can drop out of git history and trigger 30-min full reimports every cycle. The handler accepts an optional sourceId job param for callers that want to override; falls back to the resolveSourceForDir lookup when absent. CODEX-4: replace the hardcoded concurrency=4 default with the shared autoConcurrency policy. Behavior is now consistent between CLI sync, the Minion handler, and the autopilot cycle's sync phase. Jobs that request a specific concurrency via job.data.concurrency still win. noEmbed default stays at true — embed is a separate job (submit gbrain embed --stale, OR rely on the autopilot cycle's embed phase). The doc comment makes that contract explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: e2e parallel sync against real Postgres + benchmark DATABASE_URL-gated E2E coverage that PGLite-only tests can't reach: T2 — happy path: 60 files imported at concurrency=4, all 60 pages land in the DB, with a pg_stat_activity probe before/after to confirm worker engines (4 × 2 connections) actually disconnected. P4 — benchmark: 120-file fixture, serial vs concurrency=4 timing. Emits a single-line `SYNC_PARALLEL_BENCH 120 files | serial=Xms | parallel(4)=Yms | speedup=Zx` so the CHANGELOG can quote a real number instead of an unbacked '~4×' claim. Asserts parallel <= serial * 1.5 to allow for noisy CI but fail genuine regressions. Skips gracefully when DATABASE_URL is unset (consistent with the rest of test/e2e/). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: v0.22.10 release notes + sync follow-up TODO VERSION + package.json + bun.lock: 0.22.5/0.22.6 → 0.22.10. Repo had existing drift between VERSION and package.json on master; this commit brings them back in sync at the bumped value. CHANGELOG.md: v0.22.10 entry replaces the unfinished v0.23.0 stub from PR #490's original commit. Voice-rule clean (no em dashes, no AI vocabulary), real benchmark numbers from the new E2E test (serial=289ms parallel(4)=221ms speedup=1.31x), additive worker-pool note (A3), 'To take advantage of v0.22.10' self-repair block per CLAUDE.md convention. TODOS.md: A4 follow-up filed — plumb resolved database_url through SyncOpts so performSync / performFullSync / import.ts don't each call loadConfig() separately. Deferred to a future patch; not on the v0.22.10 critical path. Patch (not minor) framing held even though new CLI surface lands here; release-notes prose names the behavior change explicitly so users know to read them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: update CLAUDE.md + README for v0.22.10 sync hardening CLAUDE.md: - New "Key files" entries for src/core/sync-concurrency.ts and src/core/db-lock.ts (both v0.22.10). - New "Key files" entry for src/commands/sync.ts (covers the lock, head-drift gate, engine.kind discriminator, vanished-file failure capture, parallel branch wiring). - Updated src/commands/jobs.ts entry with v0.22.10 sourceId resolution + autoConcurrency policy + noEmbed contract. - Added test/sync-concurrency.test.ts and test/sync-parallel.test.ts to the unit-test list with case counts. - Added test/e2e/sync-parallel.test.ts to the E2E section with the SYNC_PARALLEL_BENCH grep marker for CHANGELOG quoting. - Added "Key commands added in v0.22.10" section: gbrain sync --workers, gbrain import --workers (parseWorkers validation). README.md: added --workers flag to the IMPORT section's gbrain sync and gbrain import lines, with the >100-file auto-parallelize note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version slot to v0.22.13 VERSION 0.22.10 → 0.22.13. Master moved to 0.22.8 plus claimed slots 0.22.9-0.22.12 in sibling workspaces; 0.22.13 is the next free slot for this PR's parallel-sync hardening work. Updated all v0.22.10 references in CHANGELOG.md (release header + self-repair block), TODOS.md (D-PR490-1 follow-up tag), CLAUDE.md (Key files entries + tests + commands subsection), and the inline v0.22.10 markers in src/core/sync-concurrency.ts, src/core/db-lock.ts, src/commands/sync.ts, src/commands/import.ts, src/commands/jobs.ts, test/sync-parallel.test.ts, test/e2e/sync-parallel.test.ts. No behavioral change. CHANGELOG header rewrite, content unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: regenerate llms-full.txt for v0.22.13 doc updates CI's build-llms generator test failed because llms-full.txt was stale relative to the README + CLAUDE.md updates this PR added (--workers flag in the IMPORT section, sync-concurrency.ts/db-lock.ts/sync.ts entries in the Key files section). Per CLAUDE.md: "Run \`bun run build:llms\` after adding a new doc." The test test/build-llms.test.ts:67 verifies committed bundles match generator output — now they do again. llms.txt was already in sync (no curated config additions); only llms-full.txt needed the regen. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: root <root@localhost> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
168 lines
6.0 KiB
TypeScript
168 lines
6.0 KiB
TypeScript
/**
|
||
* E2E test for parallel sync against real Postgres.
|
||
*
|
||
* T2 — happy path: 60-file sync at concurrency=4 against PostgresEngine
|
||
* actually constructs N worker engines, imports correctly, and does
|
||
* not leak connections (probe pg_stat_activity before/after).
|
||
* P4 — benchmark: serial vs concurrency=4 timing on the same fixture so
|
||
* the v0.22.13 CHANGELOG can quote a real number instead of "~4×".
|
||
*
|
||
* Gated on DATABASE_URL. Run via:
|
||
* docker run -d --name gbrain-test-pg -e POSTGRES_USER=postgres \
|
||
* -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=gbrain_test \
|
||
* -p 5435:5432 pgvector/pgvector:pg16
|
||
* DATABASE_URL=postgresql://postgres:postgres@localhost:5435/gbrain_test \
|
||
* bun test test/e2e/sync-parallel.test.ts
|
||
*/
|
||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||
import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'fs';
|
||
import { join } from 'path';
|
||
import { tmpdir } from 'os';
|
||
import { execSync } from 'child_process';
|
||
import { hasDatabase, setupDB, teardownDB, getEngine, getConn } from './helpers.ts';
|
||
|
||
const skip = !hasDatabase();
|
||
const describeE2E = skip ? describe.skip : describe;
|
||
|
||
if (skip) {
|
||
console.log('Skipping E2E sync-parallel tests (DATABASE_URL not set)');
|
||
}
|
||
|
||
function seedRepo(repoPath: string, fileCount: number): string {
|
||
execSync('git init', { cwd: repoPath, stdio: 'pipe' });
|
||
execSync('git config user.email "test@test.com"', { cwd: repoPath, stdio: 'pipe' });
|
||
execSync('git config user.name "Test"', { cwd: repoPath, stdio: 'pipe' });
|
||
mkdirSync(join(repoPath, 'people'), { recursive: true });
|
||
for (let i = 0; i < fileCount; i++) {
|
||
writeFileSync(join(repoPath, `people/p${i}.md`), [
|
||
'---',
|
||
'type: person',
|
||
`title: Person ${i}`,
|
||
'---',
|
||
'',
|
||
`Person ${i} body — some text long enough to chunk.`,
|
||
`Iteration index ${i}, generated by sync-parallel E2E.`,
|
||
].join('\n'));
|
||
}
|
||
execSync('git add -A && git commit -m "initial"', { cwd: repoPath, stdio: 'pipe' });
|
||
return execSync('git rev-parse HEAD', { cwd: repoPath, encoding: 'utf-8' }).trim();
|
||
}
|
||
|
||
async function activeConnections(): Promise<number> {
|
||
const conn = getConn();
|
||
const rows = await conn.unsafe(`
|
||
SELECT count(*) AS n FROM pg_stat_activity
|
||
WHERE datname = current_database()
|
||
AND state IS NOT NULL
|
||
`) as Array<{ n: string }>;
|
||
return parseInt(rows[0]?.n ?? '0', 10);
|
||
}
|
||
|
||
describeE2E('E2E sync-parallel: T2 happy path + leak probe', () => {
|
||
let repoPath: string;
|
||
|
||
beforeAll(async () => {
|
||
await setupDB();
|
||
});
|
||
|
||
afterAll(async () => {
|
||
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
||
await teardownDB();
|
||
});
|
||
|
||
test('60-file Postgres sync at concurrency=4 imports all + no connection leak', async () => {
|
||
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-e2e-par-'));
|
||
seedRepo(repoPath, 60);
|
||
|
||
const before = await activeConnections();
|
||
|
||
const { performSync } = await import('../../src/commands/sync.ts');
|
||
const engine = getEngine();
|
||
const result = await performSync(engine, {
|
||
repoPath,
|
||
noPull: true,
|
||
noEmbed: true,
|
||
concurrency: 4,
|
||
});
|
||
|
||
// First sync routes through performFullSync (delegates to runImport which
|
||
// also accepts --workers); status is 'first_sync'.
|
||
expect(result.status).toBe('first_sync');
|
||
|
||
const after = await activeConnections();
|
||
|
||
// Allow some slack — the helper engine + sync's normal pool stay open.
|
||
// Worker engines (4 × 2 = 8 connections) MUST have closed; if they
|
||
// hadn't, after - before would be at least 8.
|
||
expect(after - before).toBeLessThan(4);
|
||
|
||
// Verify pages are actually in the DB (via raw SQL — engine API also works).
|
||
const conn = getConn();
|
||
const pageRows = await conn.unsafe(
|
||
`SELECT count(*) AS n FROM pages WHERE slug LIKE 'people/p%'`,
|
||
) as Array<{ n: string }>;
|
||
const count = parseInt(pageRows[0]?.n ?? '0', 10);
|
||
expect(count).toBe(60);
|
||
}, 60_000);
|
||
});
|
||
|
||
describeE2E('E2E sync-parallel: P4 benchmark serial vs concurrency=4', () => {
|
||
let repoSerial: string;
|
||
let repoParallel: string;
|
||
|
||
beforeAll(async () => {
|
||
await setupDB();
|
||
});
|
||
|
||
afterAll(async () => {
|
||
if (repoSerial) rmSync(repoSerial, { recursive: true, force: true });
|
||
if (repoParallel) rmSync(repoParallel, { recursive: true, force: true });
|
||
await teardownDB();
|
||
});
|
||
|
||
test('120-file benchmark: report serial and parallel wall-clock', async () => {
|
||
// Two separate repos so neither sync's chunks bleed into the other.
|
||
repoSerial = mkdtempSync(join(tmpdir(), 'gbrain-bench-serial-'));
|
||
repoParallel = mkdtempSync(join(tmpdir(), 'gbrain-bench-parallel-'));
|
||
seedRepo(repoSerial, 120);
|
||
seedRepo(repoParallel, 120);
|
||
|
||
const { performSync } = await import('../../src/commands/sync.ts');
|
||
const engine = getEngine();
|
||
|
||
// Truncate between runs to keep the benchmark honest.
|
||
const conn = getConn();
|
||
|
||
const t1 = Date.now();
|
||
await performSync(engine, {
|
||
repoPath: repoSerial,
|
||
noPull: true,
|
||
noEmbed: true,
|
||
concurrency: 1,
|
||
});
|
||
const serialMs = Date.now() - t1;
|
||
|
||
// Wipe pages before second run so neither one is "incremental".
|
||
await conn.unsafe(`TRUNCATE pages CASCADE`);
|
||
await conn.unsafe(`TRUNCATE config CASCADE`);
|
||
|
||
const t2 = Date.now();
|
||
await performSync(engine, {
|
||
repoPath: repoParallel,
|
||
noPull: true,
|
||
noEmbed: true,
|
||
concurrency: 4,
|
||
});
|
||
const parallelMs = Date.now() - t2;
|
||
|
||
const speedup = (serialMs / parallelMs).toFixed(2);
|
||
// Emit as a single line stdout consumers can grep for.
|
||
console.log(`SYNC_PARALLEL_BENCH 120 files | serial=${serialMs}ms | parallel(4)=${parallelMs}ms | speedup=${speedup}x`);
|
||
|
||
// Soft assertion: parallel must not be slower than serial. The actual
|
||
// speedup ratio depends heavily on Postgres latency profile and is what
|
||
// the CHANGELOG quotes — don't gate the test on a specific multiplier.
|
||
expect(parallelMs).toBeLessThanOrEqual(serialMs * 1.5); // +50% slack for noisy CI
|
||
}, 120_000);
|
||
});
|