Skip to content

indexer: batch per-file DB statements in index_repo (#105) - #118

Merged
IceRhymers merged 10 commits into
integration/indexer-performancefrom
feat/105-batch-index-writes
Jul 25, 2026
Merged

indexer: batch per-file DB statements in index_repo (#105)#118
IceRhymers merged 10 commits into
integration/indexer-performancefrom
feat/105-batch-index-writes

Conversation

@IceRhymers

Copy link
Copy Markdown
Owner

Refs #105

Summary

Batches index_repo's per-file DB writes. Today each file issues 3–7
sequential round trips (file upsert, symbol delete+insert, edge
delete+insert, chunk delete+insert). This processes files in batches of up
to _BATCH_MAX_FILES=500 / _BATCH_MAX_CONTENT_BYTES inside the existing
single (repo, branch) transaction:

  • One multi-row INSERT ... ON CONFLICT ... RETURNING id, path, content_sha
    per batch for files; ids are mapped back by (path, content_sha),
    never by row order (RETURNING on a multi-row DO UPDATE makes no
    ordering guarantee) — a missing key raises and rolls back the whole
    transaction rather than risk attaching one file's rows to another.
  • One DELETE ... WHERE file_id = ANY(:ids) + one param-budgeted bulk insert
    per batch for symbols and reference_edges (new indexer/bulk.py
    helper, explicit multi-row VALUES rather than executemany so the
    round-trip count is driver-independent and measurable).
  • The chunk_writer seam reshaped from per-file to per-batch
    (indexer/chunk_store.write_chunks_batch), keeping the
    "precomputed vectors only, no network inside the transaction" contract.
  • Every ON CONFLICT DO UPDATE uses excluded.* for per-row columns, never
    a Python literal, to avoid the last-file-in-batch silently overwriting
    every conflicting row.

Acceptance criteria

  • Round trips per file 3–7 → ≲0.05. Measured (not projected):
    0.0133 statements/file on a 600-file first-time index
    (test_statement_count_per_file_meets_the_acceptance_criterion).
  • db-write phase ≥5× on a full index of a large repo. Measured
    6.38× on local Postgres (loopback, not Lakebase) — see
    docs/perf/issue-105-measurements.md §3 for the full before/after numbers
    and both caveats (round-trip latency is understated on loopback; the
    unindexed symbols.file_id seq scan is not). Carried as not ticked,
    open on epic [Epic] Indexer performance: delta indexing, batched writes, streaming ingest #110 pending a real Lakebase run.
  • Byte-identical corpus vs. the per-file path. A 3-branch,
    33-distinct-row fixture indexed identically on this branch and on
    732a7d7 (pre-indexer: batch per-file DB statements in index_repo #105), dumped and diffed field-for-field for files,
    symbols, and reference_edges — empty diff. Reproduced in
    docs/perf/issue-105-measurements.md §2, plus
    test_batch_size_invariance_produces_a_byte_identical_corpus at
    _BATCH_MAX_FILES ∈ {1, 2, 7, 500}.
  • Rollback unchanged. A mid-batch generator failure rolls back the
    whole (repo, branch) transaction
    (test_mid_batch_generator_failure_rolls_back_the_whole_transaction); CAS
    conflict still rolls back the batched form too.

Independent review

An independent code-reviewer agent pass (not the executor that wrote this
code) found two MEDIUM issues, both fixed in cc34de6 before this PR opened:

  • _union_membership had no dedup guard on the injected items seam
    (unlike _flush_file_batch's own guard), so a duplicated
    (path, content_sha) reaching the membership-only class fed the same
    file_id to write_chunks_batch twice — a real UNIQUE VIOLATION on
    uq_chunks_file_id_chunk_index that poisons the transaction. Reproduced
    against real Postgres both before and after the fix. The guard now lives
    in write_chunks_batch itself (mirrors _flush_file_batch's dedup,
    last-occurrence-wins, one WARNING), covering every caller rather than
    just this one call site.
  • The Lakebase-deferred chunk_writer stubs in
    tests/integration/test_store_chunk_writer.py and
    tests/integration/test_reconcile.py looped the old per-file
    write_chunks, so even when those modules eventually run against a real
    Lakebase branch they'd exercise the pre-indexer: batch per-file DB statements in index_repo #105 statement shape, not
    write_chunks_batch. Both now call write_chunks_batch once per batch,
    matching indexer/job.py's real closure; test_chunk_batching.py (which
    does run locally) gained membership-only and duplicate-item coverage
    reproducing the fixed regression end-to-end.

Three LOW findings were left as-is (documented, not fixed): insert_rows
assumes homogeneous row dicts with no assertion; _FILE_UPSERT_COLUMNS is a
hand-maintained constant not derived from the actual column count;
_union_membership's chunk write has no batch-size bound (unlike the
changed/new path) — not the dominant memory term today since
job._precompute_chunk_writer already retains the whole branch's
embeddings.

Gates run

  • make lint (ruff check + format + mypy): green.
  • make test (unit): 1244 passed, 1 known false positive
    test_semantics_change_bumps_the_index_semantics_version fires because
    indexer/ingest.py (added by indexer: single-pass in-memory tarball ingestion (drop extract-to-disk) #106, not yet merged past
    integration/indexer-performance) is new relative to origin/master, the
    tripwire's local fallback base. Verified green (9/9) against the correct
    sibling base: GITHUB_BASE_REF=integration/indexer-performance uv run pytest tests/unit/test_semantics_version_tripwire.py.
  • make test-integration against local codesearch-pg (the only
    integration gate available — no CI Lakebase job is provisioned
    repo-wide): all batching-relevant suites pass, including the rollback and
    statement-count tests above. The remaining failures/errors are
    lakebase_vector/lakebase_tokenizer-dependent suites this Postgres
    image can't provide, plus two pre-existing failures
    (test_commit_search.py, test_mcp_server.py) unrelated to this diff and
    reproducible on the unmodified base branch.

Known limits

  • AC2 (≥5× db-write time) not verified against real Lakebase — see above.
  • symbols has no index on file_id (pre-existing, documented in
    docs/runbooks/indexing-parallelism.md §2.3); this change reduces it to
    one sequential scan per ≤500 files instead of one per file, but doesn't
    remove it.
  • The three LOW findings above, left as documented follow-ups.

_union_membership had no guard against a duplicated (path, content_sha)
in the injected items seam, unlike _flush_file_batch's own dedup guard --
a duplicate landing in the membership-only class reached write_chunks_batch
twice for the same file_id, raising a UNIQUE VIOLATION on
uq_chunks_file_id_chunk_index and rolling back the whole branch transaction
(reproduced against real Postgres, both before and after this fix). The
guard now lives in write_chunks_batch itself, covering every caller.

Also updates the Lakebase-deferred chunk_writer stubs in
test_store_chunk_writer.py and test_reconcile.py to call write_chunks_batch
once per batch instead of looping the old per-file write_chunks, matching
indexer/job.py's real closure, and adds membership-only + duplicate-item
coverage to test_chunk_batching.py, which runs locally against codesearch-pg.

Found by an independent code-reviewer agent pass over issue #105.
@IceRhymers
IceRhymers merged commit 86f8e8b into integration/indexer-performance Jul 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant