From e70dd16ff9bc79cef5eb4b1cfbd29276e43030a9 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Sat, 5 Sep 2026 22:53:39 -0500 Subject: [PATCH] Avoid payload table scans during startup recovery --- docs/benchmarking.md | 41 ++++++++++++++++++++++++++++++++++++++--- src/storage/store.zig | 25 +++++++++++++++++++++++-- tools/startup_bench.py | 8 +++++--- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/docs/benchmarking.md b/docs/benchmarking.md index 17fc7f5..1b8462d 100644 --- a/docs/benchmarking.md +++ b/docs/benchmarking.md @@ -218,7 +218,8 @@ individual samples and min/median/max. Polling adds up to roughly 5 ms plus HTTP request overhead; these are warm-filesystem runs, not cold VM boots. The lanes separate fresh schema creation, empty existing-database restart, and -history-shaped restart. The history fixture has 10,000 commits, 60,000 blocks, +history-shaped restart. The history fixture has 10,000 commits, 60,000 blocks with 4 KiB payloads, +10,000 sequence events with 4 KiB payloads, and 2,000 unversioned tree blocks which match neither records nor commits. It is a synthetic SQLite workload, not a signed repository or an API throughput benchmark. Increase the second argument to examine scaling. @@ -237,7 +238,8 @@ logs or account data. Use local backups rather than a production database path. Startup logs report `storage_ms`, `account_setup_ms`, and `http_init_ms`. They measure storage initialization, account bootstrap/announcement setup, and HTTP -server construction respectively. The existing "listening" message precedes +server construction respectively. Storage also reports `migrations_ms` and +`sequence_ms` to separate those two phases. The existing "listening" message precedes `server.listen()`; HTTP health remains the authoritative readiness measurement. ### 2026-09-05: index commit CID lookups @@ -247,7 +249,8 @@ commits by `(did, cid)`. Without a matching index, unmatched historical tree blocks repeatedly scan an account's commit history. A non-unique composite index removes those scans without skipping backfill or integrity validation. -Five-run medians on macOS arm64, ReleaseSafe, warm filesystem: +Five-run medians on macOS arm64, ReleaseSafe, warm filesystem. This initial +fixture had block and commit history but no sequence-event payloads: | lane | before | indexed | | --- | ---: | ---: | @@ -267,3 +270,35 @@ health probe across replacement. Compare failure windows separately from healthy response latency. A single machine with a local SQLite volume has a deployment interruption even when application startup is fast; avoid extra restarts solely to publish benchmark notes. + +### sequence recovery + +The first indexed production deployment still recorded `storage_ms=5709`. +The fixture improvement alone did not establish a production improvement. +Read-only production probes then identified another scan: computing the next +sequence via `MAX` over a union of every commit and event sequence. That query +scanned the event table (including its large row storage). Computing `MAX(seq)` +inside each branch first preserves empty-table and gap semantics while letting +SQLite seek to each table's final integer primary key. A production read-only +comparison returned 10914 for both queries, taking 556 ms and 1 ms respectively; +these are individual warm query measurements, not full startup measurements. + +The history fixture now includes 10,000 events with 4 KiB payloads to exercise +this path. Regression tests cover empty histories, either history being ahead, +gaps, and an empty commit table with retained events. + +### narrow revision backfill to missing revisions + +Production `EXPLAIN QUERY PLAN` showed `SCAN repo_blocks` for both backfill +updates even after commit lookup was indexed. Unlike the read-only SELECT +probes, UPDATE selected a full scan of payload-bearing block rows. A partial +index on `(did, cid) WHERE repo_rev IS NULL` bounds subsequent backfill work to +unversioned blocks. It is created after the legacy `repo_rev` column migration. +Index creation still scans existing blocks once on upgrade; it does not remove +or bypass migration work. The fixture now includes 4 KiB block payloads as well +as events, exposing the cost of scanning payload-bearing tables. + +With payloads in the fixture, five-run macOS ReleaseSafe median readiness fell +from 33.86 ms (commit index only) to 9.22 ms (partial index plus per-table sequence +maxima). Empty restarts measured 9.16 ms. These warm-cache results are distinct +from the initial small-block comparison above. diff --git a/src/storage/store.zig b/src/storage/store.zig index 80be496..d02285d 100644 --- a/src/storage/store.zig +++ b/src/storage/store.zig @@ -586,8 +586,12 @@ pub fn init(io: Io, path: []const u8) !void { try conn.busyTimeout(5000); try conn.execNoArgs("PRAGMA journal_mode=WAL"); try conn.execNoArgs("PRAGMA foreign_keys=ON"); + const migrations_started = clock.monotonicNs(); try migrate(); + const migrations_ready = clock.monotonicNs(); next_seq = try loadNextSeqLocked(); + log.info("startup migrations_ms={d}\n", .{(migrations_ready - migrations_started) / std.time.ns_per_ms}); + log.info("startup sequence_ms={d}\n", .{(clock.monotonicNs() - migrations_ready) / std.time.ns_per_ms}); } pub fn close() void { @@ -5608,9 +5612,9 @@ fn loadNextSeqLocked() !u64 { const row = try conn.row( \\SELECT COALESCE(MAX(seq), 0) + 1 \\FROM ( - \\ SELECT seq FROM commits + \\ SELECT MAX(seq) AS seq FROM commits \\ UNION ALL - \\ SELECT seq FROM seq_events + \\ SELECT MAX(seq) AS seq FROM seq_events \\) , .{}); if (row == null) return 1; @@ -7043,6 +7047,8 @@ const post_schema_statements = [_][*:0]const u8{ \\) , "CREATE INDEX IF NOT EXISTS repo_blocks_rev_idx ON repo_blocks (did, repo_rev DESC, cid DESC)", + // Backfill must not scan payload-bearing rows whose revisions are already known. + "CREATE INDEX IF NOT EXISTS repo_blocks_missing_rev_idx ON repo_blocks (did, cid) WHERE repo_rev IS NULL", }; test "persists records in sqlite" { @@ -9075,3 +9081,18 @@ test "remembered browser credentials expire and revoke independently of OAuth" { try deleteBrowserSession("digest"); try std.testing.expect(try getBrowserSession(allocator, "digest", 1000) == null); } + +test "startup next sequence uses both histories including gaps and empty tables" { + try init(std.Options.debug_io, ":memory:"); + defer close(); + try std.testing.expectEqual(@as(u64, 1), try loadNextSeqLocked()); + try conn.execNoArgs("INSERT INTO accounts (did, handle, email, password_hash) VALUES ('did:plc:sequence', 'sequence.test', 'sequence@example.test', 'unused')"); + try conn.execNoArgs("INSERT INTO commits (seq, did, cid, rev) VALUES (7, 'did:plc:sequence', 'commit', 'rev')"); + try std.testing.expectEqual(@as(u64, 8), try loadNextSeqLocked()); + try conn.execNoArgs("INSERT INTO seq_events (seq, did, commit_cid, evt) VALUES (3, 'did:plc:sequence', 'commit', X'A0')"); + try std.testing.expectEqual(@as(u64, 8), try loadNextSeqLocked()); + try conn.execNoArgs("INSERT INTO seq_events (seq, did, commit_cid, evt) VALUES (19, 'did:plc:sequence', 'commit', X'A0')"); + try std.testing.expectEqual(@as(u64, 20), try loadNextSeqLocked()); + try conn.execNoArgs("DELETE FROM commits"); + try std.testing.expectEqual(@as(u64, 20), try loadNextSeqLocked()); +} diff --git a/tools/startup_bench.py b/tools/startup_bench.py index 3b987b6..3bfbcd6 100644 --- a/tools/startup_bench.py +++ b/tools/startup_bench.py @@ -67,9 +67,11 @@ def seed_history(db, commits): conn.execute("INSERT INTO accounts(did,handle,email,password_hash,account_status) VALUES ('did:plc:bench','bench.test','bench@example.test','unused','deactivated')") conn.executemany("INSERT INTO commits(seq,did,cid,rev) VALUES (?,'did:plc:bench',?,?)", ((i+1, f'commit-{i}', f'rev-{i}') for i in range(commits))) - conn.executemany("INSERT INTO repo_blocks(did,cid,data,repo_rev) VALUES ('did:plc:bench',?,X'A0',?)", + conn.executemany("INSERT INTO seq_events(seq,did,commit_cid,evt) VALUES (?,'did:plc:bench',?,zeroblob(4096))", + ((i+1, f'commit-{i}') for i in range(commits))) + conn.executemany("INSERT INTO repo_blocks(did,cid,data,repo_rev) VALUES ('did:plc:bench',?,zeroblob(4096),?)", ((f'commit-{i}', f'rev-{i}') for i in range(commits))) - conn.executemany("INSERT INTO repo_blocks(did,cid,data,repo_rev) VALUES ('did:plc:bench',?,X'A0',?)", + conn.executemany("INSERT INTO repo_blocks(did,cid,data,repo_rev) VALUES ('did:plc:bench',?,zeroblob(4096),?)", ((f'tree-{i}', None if i < commits//5 else 'rev-0') for i in range(commits*5))) @@ -84,7 +86,7 @@ def main(): if args.runs < 1 or args.timeout <= 0 or args.history < 0: parser.error('runs and timeout must be positive; history must be nonnegative') binary = args.binary.resolve(strict=True) - result = {'platform': platform.platform(), 'binary': str(binary), 'binary_sha256': hashlib.sha256(binary.read_bytes()).hexdigest(), 'poll_interval_ms': 5, 'history_commits': args.history, + result = {'platform': platform.platform(), 'binary': str(binary), 'binary_sha256': hashlib.sha256(binary.read_bytes()).hexdigest(), 'poll_interval_ms': 5, 'history_commits': args.history, 'block_bytes': 4096, 'event_bytes': 4096, 'note': 'warm filesystem caches; excludes build, VM boot and proxy routing', 'scenarios': {}} with tempfile.TemporaryDirectory(prefix='zds-startup-') as root: directory = Path(root) -- 2.51.2