kayg/calternal

Watch

Fork 0

PERF: publish Search generations without whole-manifest writer transactions (#663) #832

New issue

Open

opened 2026-10-02 13:22:24 +00:00 by kayg · 6 comments

kayg commented

2026-10-02 13:22:24 +00:00

Owner

Context: SQLite architecture audit #663, DESIGN §58 rule 8. Source base c4a61e8cf0 and queued merge-round-7a 2f4482ded0. Reuse #496's bounded reconcile work; do not revert its memory improvements.

Evidence: base crates/calternal-search/src/indexer.rs:2515 persist_manifest deletes the whole live manifest and inserts every row in one transaction. Batches of 1,024 rows bound statement parameters, not transaction duration. Queued merge-round-7a improves memory by staging on disk, but indexer.rs:2695–2715 publish_staged_manifest still takes the core Index writer once, clears the previous manifest, copies all live rows into previous, deletes all live rows and copies all staged rows into live, then commits. restore_previous_manifest :2720–2731 repeats a whole-table replacement. The server provides the core writer to Search, so this is shared with jobs and Security state.

Reasoned impact: each full publish has O(total searchable paths) writes and holds the only application writer connection for the complete operation. A 1M-path Home copies two 1M-row generations before releasing it. WAL readers can keep reading the previous committed state, but unrelated User mutations, job leases and Auth reads that still borrow the writer queue behind publication. Large statements and checkpoints can also add I/O contention. This is not a measured p95 regression or a claim that WAL's reader lock blocks readers. #496 bounds memory, which is necessary but does not bound writer occupancy.

Concrete fix: keep staged generations as generations; publish the new generation with a small pointer/revision transaction. Build and retire generations in bounded transactions with a durable cursor and yield between batches. Keep the previous generation until Tantivy directory publication/recovery succeeds. Pin readers to one generation, and never expose incomplete rows. A private Derived data Index is another option if it preserves access checks and recovery, but a generation flip is the smaller extension to the existing staging design. Coordinate with Search's current owner and #695.

Tests: build a large staged manifest, pause after each durable batch and issue an unrelated core writer mutation; prove that it completes between batches. Prove the final publication has a row-count-independent write set. Inject failure before/after each publish boundary and during retirement; restart must select a complete old or new generation and reconcile with Tantivy without ghost hits or lost authority state. Cover restore/retry and a second User's concurrent indexing. Extend existing Search ingest/rebuild bench with writer occupancy and interactive mutation accepted/durable latency, CPU/RSS, at least five locked HDD perf-VM samples and load.

Duplicate check: all issue titles through #800, #496 (memory), #503 (memory limits), #695 (Search first-usable path), #764 (empty queue polling). No issue specifically owns the full-manifest writer transaction that remains after #496. No product code or existing expectation changed. Non-blocking performance finding.

Context: SQLite architecture audit #663, DESIGN §58 rule 8. Source base c4a61e8cf090170f35b1bed3350d9de20c83ecd5 and queued merge-round-7a 2f4482ded066d9c5d9c59130377907f7fd2916c9. Reuse #496's bounded reconcile work; do not revert its memory improvements. Evidence: base crates/calternal-search/src/indexer.rs:2515 persist_manifest deletes the whole live manifest and inserts every row in one transaction. Batches of 1,024 rows bound statement parameters, not transaction duration. Queued merge-round-7a improves memory by staging on disk, but indexer.rs:2695–2715 publish_staged_manifest still takes the core Index writer once, clears the previous manifest, copies all live rows into previous, deletes all live rows and copies all staged rows into live, then commits. restore_previous_manifest :2720–2731 repeats a whole-table replacement. The server provides the core writer to Search, so this is shared with jobs and Security state. Reasoned impact: each full publish has O(total searchable paths) writes and holds the only application writer connection for the complete operation. A 1M-path Home copies two 1M-row generations before releasing it. WAL readers can keep reading the previous committed state, but unrelated User mutations, job leases and Auth reads that still borrow the writer queue behind publication. Large statements and checkpoints can also add I/O contention. This is not a measured p95 regression or a claim that WAL's reader lock blocks readers. #496 bounds memory, which is necessary but does not bound writer occupancy. Concrete fix: keep staged generations as generations; publish the new generation with a small pointer/revision transaction. Build and retire generations in bounded transactions with a durable cursor and yield between batches. Keep the previous generation until Tantivy directory publication/recovery succeeds. Pin readers to one generation, and never expose incomplete rows. A private Derived data Index is another option if it preserves access checks and recovery, but a generation flip is the smaller extension to the existing staging design. Coordinate with Search's current owner and #695. Tests: build a large staged manifest, pause after each durable batch and issue an unrelated core writer mutation; prove that it completes between batches. Prove the final publication has a row-count-independent write set. Inject failure before/after each publish boundary and during retirement; restart must select a complete old or new generation and reconcile with Tantivy without ghost hits or lost authority state. Cover restore/retry and a second User's concurrent indexing. Extend existing Search ingest/rebuild bench with writer occupancy and interactive mutation accepted/durable latency, CPU/RSS, at least five locked HDD perf-VM samples and load. Duplicate check: all issue titles through #800, #496 (memory), #503 (memory limits), #695 (Search first-usable path), #764 (empty queue polling). No issue specifically owns the full-manifest writer transaction that remains after #496. No product code or existing expectation changed. Non-blocking performance finding.

kayg commented

2026-10-02 13:37:10 +00:00

Author

Owner

SQLite audit coordination: #444 already owns moving Search manifest/frecency into per-User files. That is a separate physical-separation fix; #832 owns bounded generation publication and writer occupancy. Plan the two together and keep one Search adapter owner. Current server wire.rs:1268-1272 supplies the shared Db writer to the Search worker, so the source evidence here applies until #444 lands. A per-User file reduces cross-User contention, but a full copy still stalls that User's mutations and consumes HDD I/O. Do not discard #496's bounded-memory staging.

kayg referenced this issue

2026-10-02 13:39:12 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg commented

2026-10-02 13:40:31 +00:00

Author

Owner

Starting #832 on job/searchgen-832, based on c4a61e8cf0. The current docs/DESIGN.md has sections only through §57, so the issue’s §58 citation is absent from this base. I will use the Search decisions in §32 and the concrete acceptance criteria in #832 while tracing the existing implementation.

Starting #832 on job/searchgen-832, based on c4a61e8cf090170f35b1bed3350d9de20c83ecd5. The current docs/DESIGN.md has sections only through §57, so the issue’s §58 citation is absent from this base. I will use the Search decisions in §32 and the concrete acceptance criteria in #832 while tracing the existing implementation.

kayg commented

2026-10-02 13:45:13 +00:00

Author

Owner

Root cause confirmed at c4a61e8: crates/calternal-search/src/indexer.rs::rebuild_all calls persist_manifest before publishing Tantivy; persist_manifest opens one transaction, deletes all search_manifest rows, inserts the full manifest in 1,024-row statements, and commits only after every row. crates/calternal-db/src/db.rs configures writer_pool with max_connections(1), so Notes mutations on that pool wait for the whole reindex manifest transaction. I will preserve the existing bounded reconciliation and make the staged generation durable in short transactions before a small active-generation flip.

Root cause confirmed at c4a61e8: crates/calternal-search/src/indexer.rs::rebuild_all calls persist_manifest before publishing Tantivy; persist_manifest opens one transaction, deletes all search_manifest rows, inserts the full manifest in 1,024-row statements, and commits only after every row. crates/calternal-db/src/db.rs configures writer_pool with max_connections(1), so Notes mutations on that pool wait for the whole reindex manifest transaction. I will preserve the existing bounded reconciliation and make the staged generation durable in short transactions before a small active-generation flip.

kayg commented

2026-10-02 17:38:35 +00:00

Author

Owner

Regression evidence: cargo test -p calternal-search --lib note_save_completes_while_search_manifest_is_publishing -- --nocapture failed against the old publisher. The Note-shaped write did not complete before the 100,000-row manifest transaction ended (test runtime 0.86s). This confirms the shared writer is held for the whole manifest publish. I am implementing bounded generation batches and a pointer flip now.

Regression evidence: `cargo test -p calternal-search --lib note_save_completes_while_search_manifest_is_publishing -- --nocapture` failed against the old publisher. The Note-shaped write did not complete before the 100,000-row manifest transaction ended (test runtime 0.86s). This confirms the shared writer is held for the whole manifest publish. I am implementing bounded generation batches and a pointer flip now.

kayg commented

2026-10-02 18:52:54 +00:00

Author

Owner

The first full calternal-search test run found one stale unit fixture: global_search_omits_paired_sidecars_but_keeps_orphans still installed migrations 0001–0002 only, so actor startup reported no such table: search_manifest_active. The fixture now applies 0001–0004. The refreshed full suite passes: 37 unit tests, all integration/property/retrieval tests pass, and 3 model/benchmark tests remain ignored.

The first full `calternal-search` test run found one stale unit fixture: `global_search_omits_paired_sidecars_but_keeps_orphans` still installed migrations 0001–0002 only, so actor startup reported `no such table: search_manifest_active`. The fixture now applies 0001–0004. The refreshed full suite passes: 37 unit tests, all integration/property/retrieval tests pass, and 3 model/benchmark tests remain ignored.

kayg commented

2026-10-02 18:59:05 +00:00

Author

Owner

#832 report

Built

Search now stages file manifests in 1,024-row generation batches. Each transaction records a durable build or retirement cursor. The existing search_manifest read surface is an active-generation view. After the new Tantivy directory opens, one short transaction flips the active generation pointer; inactive generations are retired in bounded batches, with cleanup resumed by the Index actor after restart. A regression test reproduces the old Note-save stall and verifies the Note save completes between batches while the active view stays unchanged until pointer activation.

The publish benchmark profile records Notes save p50/p95, manifest writer-hold p50/p95 and max, reindex time, process CPU and RSS, and whether each save finishes before a later manifest batch. bench/search-publish-832.sh acquires /root/perf.lock, records load inside the lock, and runs the profile through bench/hdd-emu.sh.

Files

crates/calternal-search/migrations/0004_search_manifest_generations.sql
crates/calternal-search/src/lib.rs
crates/calternal-search/src/indexer.rs
crates/calternal-search/tests/ask_evaluation.rs
crates/calternal-search/tests/indexer.rs
crates/calternal-search/tests/relevance.rs
tests/perf/search_scale.py
bench/search-publish-832.sh

Commits

ea878bb43 Publish Search manifests by generation to shorten writer holds
86bcffe76 Benchmark Note saves during Search manifest publication
a9c573583 Use generation migrations in the sidecar fixture
Head: a9c57358378d2b7f4a4fb195f3976f97d6349a25

Gates

cargo fmt --check: exit 0, no output.
cargo clippy -p calternal-search --all-targets -- -D warnings (exit 0): Finished \dev` profile [unoptimized + debuginfo] target(s) in 1m 06s`
cargo test -p calternal-search (exit 0):
- test result: ok. 37 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 24.70s
- test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.05s
- test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s
- test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.83s
- test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s
- test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
- test result: ok. 1 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 15.48s
- test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
- test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
cargo clean: Removed 7272 files, 3.8GiB total (exit 0). No web build output existed.

Performance gap

The required five-sample before/after HDD measurements on root@10.69.69.63 were not completed. The baseline release build had just started locally when the owner’s four-hour job limit was reached; I stopped it (exit 130) before copying or measuring either binary. The new profile is ready, but there are no measured p50/p95, writer-hold, CPU or RSS results to report. The existing shared release binary was not used because its source revision was not verified.

Decisions

The supplied checkout and fetched origin/dev end at DESIGN §57; §58 is absent. I followed the issue’s concrete requirements and DESIGN §32.
Reused the existing 1,024-row update batch size. Kept search_manifest as a read-compatible view; actor writes address the active generation explicitly.
Restart cleanup runs on the low-priority Index actor, so manifest loading and Search startup do not wait for old generations to be deleted.

UX gaps

Closed: N/A (backend-only change).
Left: N/A (backend-only change).

## #832 report ### Built Search now stages file manifests in 1,024-row generation batches. Each transaction records a durable build or retirement cursor. The existing `search_manifest` read surface is an active-generation view. After the new Tantivy directory opens, one short transaction flips the active generation pointer; inactive generations are retired in bounded batches, with cleanup resumed by the Index actor after restart. A regression test reproduces the old Note-save stall and verifies the Note save completes between batches while the active view stays unchanged until pointer activation. The publish benchmark profile records Notes save p50/p95, manifest writer-hold p50/p95 and max, reindex time, process CPU and RSS, and whether each save finishes before a later manifest batch. `bench/search-publish-832.sh` acquires `/root/perf.lock`, records load inside the lock, and runs the profile through `bench/hdd-emu.sh`. ### Files - `crates/calternal-search/migrations/0004_search_manifest_generations.sql` - `crates/calternal-search/src/lib.rs` - `crates/calternal-search/src/indexer.rs` - `crates/calternal-search/tests/ask_evaluation.rs` - `crates/calternal-search/tests/indexer.rs` - `crates/calternal-search/tests/relevance.rs` - `tests/perf/search_scale.py` - `bench/search-publish-832.sh` ### Commits - `ea878bb43` Publish Search manifests by generation to shorten writer holds - `86bcffe76` Benchmark Note saves during Search manifest publication - `a9c573583` Use generation migrations in the sidecar fixture - Head: `a9c57358378d2b7f4a4fb195f3976f97d6349a25` ### Gates - `cargo fmt --check`: exit 0, no output. - `cargo clippy -p calternal-search --all-targets -- -D warnings` (exit 0): `Finished \`dev\` profile [unoptimized + debuginfo] target(s) in 1m 06s` - `cargo test -p calternal-search` (exit 0): - `test result: ok. 37 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 24.70s` - `test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.05s` - `test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s` - `test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.83s` - `test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s` - `test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s` - `test result: ok. 1 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 15.48s` - `test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s` - `test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s` - `cargo clean`: `Removed 7272 files, 3.8GiB total` (exit 0). No web build output existed. ### Performance gap The required five-sample before/after HDD measurements on `root@10.69.69.63` were not completed. The baseline release build had just started locally when the owner’s four-hour job limit was reached; I stopped it (exit 130) before copying or measuring either binary. The new profile is ready, but there are no measured p50/p95, writer-hold, CPU or RSS results to report. The existing shared release binary was not used because its source revision was not verified. ### Decisions - The supplied checkout and fetched `origin/dev` end at DESIGN §57; §58 is absent. I followed the issue’s concrete requirements and DESIGN §32. - Reused the existing 1,024-row update batch size. Kept `search_manifest` as a read-compatible view; actor writes address the active generation explicitly. - Restart cleanup runs on the low-priority Index actor, so manifest loading and Search startup do not wait for old generations to be deleted. ### UX gaps - Closed: N/A (backend-only change). - Left: N/A (backend-only change).

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/calternal#832

No description provided.

Rows
Columns