perf: optimize database refresh with drop-and-rebuild indexes#130
Conversation
Three optimizations cut database refresh (bulk insert) time by 21–49% across all repositories: 1. Drop and rebuild indexes around bulk inserts — indexes are dropped before the insert loop and rebuilt in one efficient pass afterward, instead of being updated per-row. This is the primary win. 2. Remove redundant pfx2as indexes — idx_pfx2as_prefix_str (redundant with the BLOB range index) and idx_pfx2as_validation (3-value enum not worth indexing). lookup_exact rewritten to use prefix_start/ prefix_end/prefix_length BLOB range index. 3. Fix PRAGMA restore bug — store() methods no longer toggle synchronous/journal_mode PRAGMAs, preserving the connection's WAL/NORMAL defaults. Previously every refresh left the connection in DELETE/FULL mode, degrading query performance until restart. Also switches INSERT OR REPLACE to plain INSERT (tables are cleared before insert) and adds a db_refresh_bench example for measuring refresh performance with real production data. Performance (real data, release build): asinfo: 988ms → 330ms (67% faster) as2rel: 2230ms → 1765ms (21% faster) pfx2as: 8631ms → 4433ms (49% faster) rpki: 71ms → 66ms (already fast) All queries remain well under 1 second on 1.6M rows.
There was a problem hiding this comment.
Pull request overview
Improves SQLite refresh performance in the database/monocle repositories by dropping/rebuilding indexes around bulk inserts, removing low-value pfx2as indexes (and updating lookup_exact accordingly), and adding a benchmark + investigation notes to quantify the impact.
Changes:
- Drop indexes before bulk insert and rebuild after across ASInfo / AS2Rel / RPKI / Pfx2as refresh paths.
- Remove
idx_pfx2as_prefix_str+idx_pfx2as_validationand rewritelookup_exactto use(prefix_start, prefix_end, prefix_length)matching. - Add
db_refresh_benchexample plus documentation and changelog entry for the performance work.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/database/monocle/rpki.rs | Drops/rebuilds RPKI indexes around refresh; adds tests asserting indexes exist after store. |
| src/database/monocle/pfx2as.rs | Removes two indexes, updates lookup_exact to use BLOB/range columns, drops/rebuilds remaining indexes around refresh, adds tests. |
| src/database/monocle/asinfo.rs | Drops/rebuilds ASInfo indexes around JSONL refresh; switches to plain INSERT; adds tests for index rebuild. |
| src/database/monocle/as2rel.rs | Drops/rebuilds AS2Rel indexes around refresh; switches to plain INSERT; adds an index rebuild test (but not exercising load_from_path). |
| examples/db_refresh_bench.rs | Adds a refresh/query benchmark example (needs updates to match new pfx2as defaults and PRAGMA behavior). |
| docs/db-refresh-perf-investigation.md | Adds written investigation notes and before/after measurements. |
| CHANGELOG.md | Documents refresh performance improvements and PRAGMA behavior fix. |
| Cargo.toml | Registers the new db_refresh_bench example target. |
Comments suppressed due to low confidence (2)
src/database/monocle/pfx2as.rs:285
- Manual
BEGIN .../COMMITmeans any early-return error before COMMIT will leave the connection inside an open transaction (potentially locking the DB for subsequent calls). Using arusqlite::Transaction(e.g.,unchecked_transaction()) would ensure an automatic rollback on drop when errors propagate.
// Begin transaction - all changes are atomic, data remains accessible until commit
self.conn.execute("BEGIN IMMEDIATE TRANSACTION", [])?;
src/database/monocle/rpki.rs:371
- Manual
BEGIN TRANSACTION/COMMITcan leave the connection in an open transaction if an error occurs before COMMIT (locking the DB for future operations). Prefer arusqlite::Transactionso rollback happens automatically on error paths.
// Begin transaction for batch insert
self.conn.execute("BEGIN TRANSACTION", [])?;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move DROP INDEX + CREATE INDEX inside the transaction in all 4 repositories so refresh is atomic: either data+indexes commit together, or everything rolls back to the previous indexed state. - Switch pfx2as and rpki from manual BEGIN/COMMIT to unchecked_transaction() for automatic rollback on error. - Fix as2rel test to call load_from_path() instead of manually dropping/recreating indexes. - Update benchmark comments to reflect current code (3 indexes is the default, no PRAGMA toggles, lookup_exact uses BLOB range index).
|
Addressed the review comments in f1dfd0f:
Validation run locally: |
|
Addressed the latest review comments in a16b9a0:
Validation run locally:
Ready for re-review. |
| // Everything below is in one transaction so failures roll back to the | ||
| // previous state (with original indexes intact). | ||
| let tx = self | ||
| .conn | ||
| .unchecked_transaction() | ||
| .map_err(|e| anyhow!("Failed to begin transaction: {}", e))?; |
| /// Index creation SQL (rebuilt after bulk insert) | ||
| const INDEX_SQL: [&'static str; 2] = [ | ||
| "CREATE INDEX IF NOT EXISTS idx_as2rel_asn1 ON as2rel(asn1)", | ||
| "CREATE INDEX IF NOT EXISTS idx_as2rel_asn2 ON as2rel(asn2)", | ||
| ]; |
Resolve merge conflicts between SSE service overhaul and db-refresh-optimization: Cargo.toml: keep database example from sse, add db_refresh_bench from main, remove stale ws_client_all reference (file deleted in sse overhaul). CHANGELOG.md: merge Breaking Changes (SSE) + Performance/Code Improvements (refresh) + New Features (SSE) under Unreleased changes. Review fix: remove unnecessary As2relSortOrder re-sort in AS2REL search endpoint that overrode the user's sort_by_asn flag. Review fix: remove misleading /rpki/aspa/validate endpoint from rpki module docs (deferred feature). Review fix: handle pfx2as refresh early in database_refresh with a clear message instead of returning a 500 InternalError from spawn_blocking. Review fix: install curl in Docker runtime image for healthcheck. Review fix: use constant-time byte comparison for bearer token auth to reduce timing side-channels. Review fix: update database README to note pfx2as HTTP API refresh is not yet implemented.
Summary
idx_pfx2as_prefix_strand low-valueidx_pfx2as_validationindexes; rewritelookup_exactto use the BLOB range indexstore()no longer togglessynchronous/journal_mode, preservingWAL/NORMALconnection defaultsINSERT OR REPLACE→ plainINSERT(tables cleared before insert)db_refresh_benchexample for measuring refresh with real dataPerformance (real data, release build)
Query performance (pfx2as, 1.6M rows)
All queries well under the 1-second target:
PRAGMA state
Test coverage
lookup_exacttests (IPv4/IPv6, multi-ASN, no-match, prefix-length distinction, consistency cross-check, error handling)test_store_rebuilds_indexes_correctlytests (verify indexes exist after store)test_store_idempotent_index_rebuildtests (no duplicate indexes on re-store)How to benchmark
Full investigation details in
docs/db-refresh-perf-investigation.md.