design(storage): decide where runtime.sqlite's one-time full VACUUM runs before converting it to incremental vacuum #6000

Activity

1. garvit-arora commented on Oct 8, 2026

   

   garvit-arora

   on Oct 8, 2026

   Contributor

   More actions

   I'd like to help investigate this design decision. I can compare the worker-thread, no-client, pre-listen, and report-only options against the current Host/storage lifecycle, then document a recommendation with explicit locking, deadline, and recovery trade-offs. I won't start an implementation until the placement direction is confirmed.
2. liugddx commented on Oct 9, 2026

   

   liugddx

   on Oct 9, 2026

   MemberAuthor

   More actions

   Thanks @garvit-arora, assigning this to you for the investigation, and agreed: no implementation until the placement is decided. What would let a maintainer decide:

   • For each option (worker thread with its own connection, run only with no client attached, pre-listen with a startup progress state, report-only), how it interacts with the Host writer, the 75 s startup deadline and the 8 s client liveness timeout. Also cover what happens on crash or power loss mid-VACUUM and on insufficient disk.
   • Measurements at a few database sizes (for example 100 MB, 1 GB and 3 GB) on at least two platforms, reusing the harness from the earlier feat(storage): reclaim context-offload SQLite free pages in Host maintenance #5855 revisions where you can.
   • A recommendation, plus the smallest first PR it implies.

   Post it as a comment here (or a short design doc linked from here), and I'll review it.
3. liugddx

   assigned

   garvit-arora

   on Oct 9, 2026
4. garvit-arora commented on Oct 9, 2026

   

   garvit-arora

   on Oct 9, 2026

   Contributor

   More actions

   Investigation PR is up: #6034. The linked design doc covers all four placement options against Host writer ownership, the 75 s startup deadline and 8 s client liveness, crash/power-loss and insufficient-disk behavior, recommendation, and smallest first PR. It includes locally measured 100 MiB / 1 GiB / 3 GiB Windows runs plus the historical #5855 production-derived 1 GiB figures, with a reproducible Ubuntu/Windows workflow. I could not dispatch that workflow yet because GitHub requires a workflow to exist on the fork's default branch; I have kept the Ubuntu result explicitly unclaimed in the doc rather than presenting synthetic Windows data as cross-platform evidence. Once the workflow is runnable, I will append the Ubuntu measurements.
5. garvit-arora commented on Oct 10, 2026

   

   garvit-arora

   on Oct 10, 2026

   Contributor

   More actions

   Thanks for the review. I corrected the benchmark and moved the placement notes here. No placement for converting existing databases is recommended yet: the requested second-platform measurement and a process-wide ownership/fencing design remain open.

   Ownership and what a worker changes

   runtime.sqlite has one process-local owner connection; repositories borrow leases. Secondary processes can also open it with require_current, so a Host-only gate does not provide cross-process exclusivity. A worker with its own second connection departs from this ownership model. The worker only keeps the Host event loop responsive and can preserve reads where SQLite permits; VACUUM remains a writer for its duration, and concurrent writers can still wait or receive SQLITE_BUSY after the configured 5 s timeout. It does not make writes safe by itself.

   Placement choices

   Choice	Host writer and clients	75 s startup / 8 s liveness	Crash and disk
   Worker after Ready	A second connection breaks the owner/lease model; a Host-only write gate misses secondary processes. SQLite still serializes writers, so concurrent writes can hit the 5 s busy timeout.	Keeps work out of startup and the event loop; reads/liveness can continue. It does not remove writer contention.	With WAL and synchronous=FULL, an uncommitted VACUUM is discarded on recovery and the original database survives. Plan close to 3x database size plus headroom: the WAL can grow to roughly database size while VACUUM also needs temporary space.
   Run only with no client attached	No attached client does not mean no Host writer: maintenance, scheduled work, cleanup, reconnect, and secondary processes can still write. New clients and writes still need a real cross-process fence.	Avoids blocking attached clients at that moment, but does not guarantee a short run or prevent reconnects. A blocked event loop can still exceed the 8 s liveness timeout after clients return.	Same transactional recovery and disk requirements as worker VACUUM. An interruption leaves the request for a later eligible window.
   Before listen with progress	No clients are attached, but startup migrations and secondary processes still need coordinated ownership.	The old production-shaped #5855 result was about 55 s for 1 GiB, consuming most of the 75 s budget. The corrected synthetic 3 GiB run is 64.150 s; progress UI does not extend the deadline.	Same recovery and roughly 3x space planning. On failure, startup must retain the request and recover/retry before listening.
   Report only	No writer lock or cross-process maintenance gate is needed. Reports reclaimable/freelist bytes but cannot compact an existing auto_vacuum=NONE database.	No startup or liveness effect.	No VACUUM crash or temporary-space risk; existing space is not reclaimed.

   A VACUUM INTO staging copy is another route for existing databases: it reads the live database and writes a separate file, leaving the live file untouched if the process crashes. Validate the staged file, then promote it at the next startup before listen. Do not swap it live on Windows while database handles are open. Promotion needs its own durable recovery state and coordination with every process that can open the database; Maka already uses a private copy for context offload (context-offload-snapshot.ts:163-166).

   Corrected synthetic measurements

   The harness now includes connection close() (and its final checkpoint) in elapsed time, then checks auto_vacuum = 2, freelist_count = 0, and integrity_check = 'ok':

   Fixture	Source bytes before deletion	Windows conversion + close
   100 MiB	106,504,192	1.384 s
   1 GiB	1,090,572,288	16.397 s
   3 GiB	3,271,692,288	64.150 s

   Windows x64, Node v24.15.0, SQLite 3.51.3 (2026-10-10). All checks passed. The prior 36.18 s 3 GiB number was the excluded DELETE-mode result. The earlier WAL 3 GiB result of 140.36 s was before close and is superseded. The corrected 1-to-3 GiB increase is still super-linear. WAL/journal I/O, SQLite page-cache behavior, storage and antivirus scanning are plausible contributors, but this fixture cannot attribute the increase to any one factor. Only Windows was measured; the requested second-platform run is still outstanding.

   Recommendation and smallest first PR

   First make new runtime.sqlite files use auto_vacuum=INCREMENTAL before schema creation and WAL activation, as the context-offload database initializer already does. This needs no full VACUUM and avoids a conversion-placement decision for new installations. Keep existing databases on report-only behavior until the second-platform measurements and cross-process ownership/recovery design are settled. For a later existing-database conversion, evaluate VACUUM INTO plus next-start promotion against a fully fenced owner-connection approach; do not add a worker-owned second SQLite connection or rely on “no client attached” as exclusivity.

   The smallest first PR is therefore only the new-database initialization change with a focused test that verifies the pragma is set before schema/WAL setup. Existing-database conversion, startup swapping, UI/protocol work, and compatibility-epoch claims stay out of that PR; claim the next free epoch only when the implementation is ready to merge.
6. garvit-arora commented on Oct 10, 2026

   

   garvit-arora

   on Oct 10, 2026

   Contributor

   More actions

   One detail on the staging alternative: VACUUM INTO operates from a read transaction on the live database and writes the compacted result to a separate staging file. A crash during that operation leaves the live database untouched; startup promotion would then need a durable marker and integrity validation. On Windows, promotion must wait until the next start before any process opens the live file.
7. liugddx commented on Oct 10, 2026

   

   liugddx

   on Oct 10, 2026

   MemberAuthor

   More actions

   Thanks @garvit-arora, this is the analysis the decision needed, and the corrected harness (close included, then asserting auto_vacuum = 2, an empty freelist and integrity_check = ok) makes the numbers trustworthy. Agreed on the recommendation, so here is the direction:

   1. Next, go ahead with the small PR: create new runtime.sqlite databases with auto_vacuum = INCREMENTAL before schema creation and WAL activation, mirroring sqlite-context-offload-schema.ts:218-223. Add a focused test that a fresh database reports auto_vacuum = 2, and that an existing NONE database is left untouched. No protocol, UI or epoch change. Once a new database is incremental, a bounded incremental_vacuum lane like the one feat(storage): reclaim context-offload SQLite free pages in Host maintenance #5855 added for context-offload can follow as its own PR.
   2. Existing databases stay report-only for now. No worker connection, and no "no client attached" exclusivity. If conversion is picked up later, the candidate to evaluate is VACUUM INTO a staging file, validated and promoted at the next start before listen, with a durable marker and coordination with every process that can open the file. That later work also needs the second-platform measurement.

   I'll keep this issue open for the existing-database decision, and the new-database PR can say Refs #6000.

   ❤️React with ❤️1Reacted by Garvit Arora
8. liugddx

   added a commit that references this issue on Oct 12, 2026

   feat(storage): initialize new runtime databases for incremental vacuum (

   ...

   105302c