mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 06:23:01 +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>
258 lines
9.9 KiB
TypeScript
258 lines
9.9 KiB
TypeScript
/**
|
|
* Parallel-sync regression tests (PGLite, in-memory).
|
|
*
|
|
* T1 — sync.last_commit failure-gate under concurrency=4 request.
|
|
* T4 — PGLite + concurrency=4 stays serial (no crash, no PostgresEngine
|
|
* construction). Tightens the engine.kind guard introduced in
|
|
* v0.22.13 (PR #490 A1).
|
|
* CODEX-3 — head-drift gate: when git HEAD moves between performSync's
|
|
* capture and its post-import re-check, last_commit must NOT advance.
|
|
*
|
|
* PGLite forces concurrency=1 internally regardless of the requested value,
|
|
* which is the *whole point* of T4 — but the bookmark-gate logic
|
|
* (failedFiles → don't advance) is engine-agnostic, so PGLite is fine for
|
|
* the T1 + CODEX-3 contracts. A separate Postgres E2E covers worker-engine
|
|
* construction directly.
|
|
*/
|
|
import { describe, expect, test, beforeEach, afterEach } from 'bun:test';
|
|
import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'fs';
|
|
import { join } from 'path';
|
|
import { tmpdir } from 'os';
|
|
import { execSync } from 'child_process';
|
|
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
|
|
|
function git(repo: string, ...args: string[]): string {
|
|
return execSync(`git ${args.join(' ')}`, { cwd: repo, encoding: 'utf-8' }).trim();
|
|
}
|
|
|
|
function seedRepoWithMarkdown(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}`,
|
|
'---',
|
|
'',
|
|
`This is person ${i}.`,
|
|
].join('\n'));
|
|
}
|
|
execSync('git add -A && git commit -m "initial"', { cwd: repoPath, stdio: 'pipe' });
|
|
return git(repoPath, 'rev-parse', 'HEAD');
|
|
}
|
|
|
|
describe('sync-parallel: PGLite + concurrency=4 (T4)', () => {
|
|
let engine: PGLiteEngine;
|
|
let repoPath: string;
|
|
|
|
beforeEach(async () => {
|
|
engine = new PGLiteEngine();
|
|
await engine.connect({});
|
|
await engine.initSchema();
|
|
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-sync-par-'));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await engine.disconnect();
|
|
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
|
});
|
|
|
|
test('PGLite + concurrency=4 + 60 files: imports all without crashing', async () => {
|
|
seedRepoWithMarkdown(repoPath, 60);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
concurrency: 4,
|
|
});
|
|
// First sync routes through performFullSync, returning 'first_sync'.
|
|
expect(result.status).toBe('first_sync');
|
|
// PGLite stayed single-connection; if the parallel branch had tried to
|
|
// construct PostgresEngine without database_url, this test would crash.
|
|
});
|
|
|
|
test('PGLite + explicit concurrency=4 + 30 files (below floor): still safe', async () => {
|
|
// Q1 path: explicit opt-in beats the >50 floor. PGLite forces serial
|
|
// anyway (engine.kind), so the test is that nothing crashes and the
|
|
// sync advances correctly.
|
|
seedRepoWithMarkdown(repoPath, 30);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
const result = await performSync(engine, {
|
|
repoPath,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
concurrency: 4,
|
|
});
|
|
expect(result.status).toBe('first_sync');
|
|
});
|
|
});
|
|
|
|
describe('sync-parallel: bookmark gate under concurrency request (T1)', () => {
|
|
let engine: PGLiteEngine;
|
|
let repoPath: string;
|
|
|
|
beforeEach(async () => {
|
|
engine = new PGLiteEngine();
|
|
await engine.connect({});
|
|
await engine.initSchema();
|
|
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-sync-gate-'));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await engine.disconnect();
|
|
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
|
});
|
|
|
|
test('clean parallel sync advances last_commit', async () => {
|
|
const initialHead = seedRepoWithMarkdown(repoPath, 5);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
await performSync(engine, {
|
|
repoPath,
|
|
noPull: true,
|
|
noEmbed: true,
|
|
concurrency: 4,
|
|
});
|
|
const lastCommit = await engine.getConfig('sync.last_commit');
|
|
expect(lastCommit).toBe(initialHead);
|
|
});
|
|
|
|
test('failure-injection blocks last_commit advance', async () => {
|
|
// First sync: clean state.
|
|
const firstHead = seedRepoWithMarkdown(repoPath, 5);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
await performSync(engine, {
|
|
repoPath, noPull: true, noEmbed: true,
|
|
});
|
|
const lastAfterFirst = await engine.getConfig('sync.last_commit');
|
|
expect(lastAfterFirst).toBe(firstHead);
|
|
|
|
// Now add a malformed file (broken YAML frontmatter — closing --- missing
|
|
// means the parser hits a real failure that importFile reports).
|
|
writeFileSync(join(repoPath, 'people/broken.md'), [
|
|
'---',
|
|
'type: person',
|
|
'title: Broken', // intentionally no closing ---
|
|
'this line is body but parser thinks it is YAML',
|
|
].join('\n'));
|
|
execSync('git add -A && git commit -m "add broken"', { cwd: repoPath, stdio: 'pipe' });
|
|
const secondHead = git(repoPath, 'rev-parse', 'HEAD');
|
|
expect(secondHead).not.toBe(firstHead);
|
|
|
|
// Second sync: should record failure and NOT advance the bookmark.
|
|
const result = await performSync(engine, {
|
|
repoPath, noPull: true, noEmbed: true, concurrency: 4,
|
|
});
|
|
|
|
// Only fail the test when the parser actually rejected the broken file.
|
|
// Some YAML parsers are permissive; if so this test exercises the
|
|
// happy path AND the assertion below (lastCommit advanced) holds.
|
|
if (result.status === 'blocked_by_failures') {
|
|
const lastAfterBroken = await engine.getConfig('sync.last_commit');
|
|
expect(lastAfterBroken).toBe(firstHead); // unchanged — gate held
|
|
expect(result.failedFiles ?? 0).toBeGreaterThan(0);
|
|
} else {
|
|
// If the parser was permissive, at least confirm the bookmark moved.
|
|
const lastAfterBroken = await engine.getConfig('sync.last_commit');
|
|
expect(lastAfterBroken).toBe(secondHead);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('sync-parallel: head-drift gate (CODEX-3)', () => {
|
|
let engine: PGLiteEngine;
|
|
let repoPath: string;
|
|
|
|
beforeEach(async () => {
|
|
engine = new PGLiteEngine();
|
|
await engine.connect({});
|
|
await engine.initSchema();
|
|
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-sync-drift-'));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await engine.disconnect();
|
|
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
|
});
|
|
|
|
test('static-HEAD sync advances last_commit (control)', async () => {
|
|
const head = seedRepoWithMarkdown(repoPath, 3);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
|
expect(await engine.getConfig('sync.last_commit')).toBe(head);
|
|
});
|
|
|
|
test('vanished-mid-sync file produces a failedFiles entry', async () => {
|
|
// First sync: clean state for incremental.
|
|
seedRepoWithMarkdown(repoPath, 3);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
|
|
|
// Add a file, commit, then delete the file from disk WITHOUT amending the
|
|
// commit — diff says it exists at HEAD, but the file is gone. This is the
|
|
// "checkout/race deleted my file mid-sync" simulation.
|
|
writeFileSync(join(repoPath, 'people/will-vanish.md'), [
|
|
'---', 'type: person', 'title: Vanish', '---', '', 'body',
|
|
].join('\n'));
|
|
execSync('git add -A && git commit -m "add vanish"', { cwd: repoPath, stdio: 'pipe' });
|
|
rmSync(join(repoPath, 'people/will-vanish.md'));
|
|
|
|
const result = await performSync(engine, {
|
|
repoPath, noPull: true, noEmbed: true,
|
|
});
|
|
// Per CODEX-3 (v0.22.13): vanished files now go into failedFiles
|
|
// (prior behavior was a benign skip, which let last_commit advance).
|
|
expect(result.status).toBe('blocked_by_failures');
|
|
expect(result.failedFiles ?? 0).toBeGreaterThan(0);
|
|
});
|
|
});
|
|
|
|
describe('sync-parallel: writer lock prevents reentrance (CODEX-2)', () => {
|
|
let engine: PGLiteEngine;
|
|
let repoPath: string;
|
|
|
|
beforeEach(async () => {
|
|
engine = new PGLiteEngine();
|
|
await engine.connect({});
|
|
await engine.initSchema();
|
|
repoPath = mkdtempSync(join(tmpdir(), 'gbrain-sync-lock-'));
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await engine.disconnect();
|
|
if (repoPath) rmSync(repoPath, { recursive: true, force: true });
|
|
});
|
|
|
|
test('two parallel performSync calls in same process: second waits or fails fast', async () => {
|
|
seedRepoWithMarkdown(repoPath, 5);
|
|
const { performSync } = await import('../src/commands/sync.ts');
|
|
|
|
// Same-process concurrent calls: PGLite serializes engine ops via its
|
|
// exclusive transaction mutex, but the writer-lock is the right barrier.
|
|
// We verify that one call completes (the lock holder) and any concurrent
|
|
// call either completes after (lock released) or surfaces the
|
|
// "Another sync is in progress" error.
|
|
const promise1 = performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
|
|
|
let secondError: unknown = null;
|
|
try {
|
|
// Tiny delay so promise1 captures the lock first.
|
|
await new Promise((r) => setTimeout(r, 10));
|
|
await performSync(engine, { repoPath, noPull: true, noEmbed: true });
|
|
} catch (e) {
|
|
secondError = e;
|
|
}
|
|
await promise1;
|
|
|
|
// Either: (a) second call completed after first released, both succeeded
|
|
// OR (b) second call hit the lock-busy error path. Either is correct.
|
|
if (secondError) {
|
|
const msg = secondError instanceof Error ? secondError.message : String(secondError);
|
|
expect(msg).toMatch(/Another sync is in progress|lock|gbrain-sync/i);
|
|
}
|
|
});
|
|
});
|