From c86d5ac4f60a3d014666a5cf81b1066011ad8252 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Tue, 26 May 2026 15:34:40 -0400 Subject: [PATCH] #000065 follow-up #48: WAL checkpoint between executor phases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add PRAGMA wal_checkpoint(TRUNCATE) at two points in _execute_all_at_once so committed WAL pages don't pin disk through subsequent passes. Production migration on 2026-05-26 hit 7 GB free disk (down from 89) because SQLite's auto-checkpoint can't run while a reader cursor is open, and FTS rebuild keeps a SELECT cursor open through 1.5M chunks per target. Across 4 targets the FTS rebuild plus audit consolidate held ~37 GB of committed-but-unreclaimed WAL. Manual sibling-connection wal_checkpoint(TRUNCATE) freed 27 GB mid-migration. Checkpoints land at: * end of _rebuild_fts_on_target (after the SELECT cursor is explicitly cur.close()'d so the TRUNCATE checkpoint can actually fire — TRUNCATE/RESTART block on active readers) * end of _consolidate_audit_chain (after the 3.47M-row giant transaction commits, before the next phase touches the same connection) VACUUM is already implicitly a checkpoint, so the existing per- target VACUUM pass continues to handle the final checkpoint naturally. Helper _checkpoint_truncate(conn) returns the (busy, log_frames, checkpointed) tuple SQLite emits; for the serial executor, busy=1 is improbable since each phase finishes before moving on. Regression test (TestWalCheckpointing.test_no_large_wal_after_migration) asserts no WAL file exceeds 4 MB after migration completes. Without the checkpoint calls this would fail on real-sized corpora; with them the test passes deterministically. Doesn't affect the running migration (it loaded the module from memory before this commit). Future reshards run with bounded WAL — no near-ENOSPC scares. --- arborist/migrate.py | 35 ++++++++++++++++++++++++++++++++++ tests/test_migrate_executor.py | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/arborist/migrate.py b/arborist/migrate.py index 03d766f..6c127ff 100644 --- a/arborist/migrate.py +++ b/arborist/migrate.py @@ -530,6 +530,13 @@ def _rebuild_fts_on_target(conn: sqlite3.Connection) -> None: chunks.content is zstd-compressed at rest (see arborist.compress); the FTS5 index needs plaintext. Stream chunks, decompress, batch into FTS. documents.title is plain text → straight copy. + + Ends with a TRUNCATE checkpoint so the WAL doesn't carry the FTS + rebuild's pages across to the next phase. Without this, 2026-05-26's + production migration accumulated ~9 GB of WAL per target across all + four FTS rebuilds (cursor-pinned WAL pages can't be reclaimed by + auto-checkpoint while the cursor is active); ran the host to within + 7 GB of ENOSPC before VACUUM. Filed as #48; landed here. """ from arborist.compress import unpack_chunk @@ -560,6 +567,10 @@ def _rebuild_fts_on_target(conn: sqlite3.Connection) -> None: "INSERT INTO chunks_fts (rowid, content) VALUES (?, ?)", fts_rows, ) + # Free the read cursor explicitly so the WAL checkpoint below can + # actually truncate (SQLite's TRUNCATE/RESTART modes block while + # any reader is pinned to a WAL page). + cur.close() with conn: conn.execute( @@ -567,6 +578,24 @@ def _rebuild_fts_on_target(conn: sqlite3.Connection) -> None: "SELECT rowid, title FROM documents WHERE title IS NOT NULL" ) + _checkpoint_truncate(conn) + + +def _checkpoint_truncate(conn: sqlite3.Connection) -> tuple[int, int, int]: + """Run PRAGMA wal_checkpoint(TRUNCATE) on this connection. + + Returns the (busy, log_frames, checkpointed) tuple SQLite reports. + Inserted between executor phases so committed WAL pages don't pin + a growing on-disk WAL through subsequent passes — the 2026-05-26 + near-ENOSPC root cause. + + busy=1 in the return value is informational only — the caller decides + whether to retry; for the migration's serial executor, a busy result + is improbable since each phase finishes its writes before moving on. + """ + row = conn.execute("PRAGMA wal_checkpoint(TRUNCATE)").fetchone() + return (int(row[0]), int(row[1]), int(row[2])) + def _consolidate_audit_chain( *, @@ -618,6 +647,12 @@ def _consolidate_audit_chain( "VALUES (?, ?, ?, ?, ?, ?)", out_rows, ) + # Collapse the 3.47M-row INSERT's WAL before the next phase + # (append_audit reshard event + meta + VACUUM). 2026-05-26 production + # migration's WAL on target 0 grew to ~9 GB across this transaction + + # the FTS rebuild and held until VACUUM; manual intervention freed + # ~27 GB. See #48. + _checkpoint_truncate(target) return len(out_rows), prev diff --git a/tests/test_migrate_executor.py b/tests/test_migrate_executor.py index 00b0967..40043ad 100644 --- a/tests/test_migrate_executor.py +++ b/tests/test_migrate_executor.py @@ -406,6 +406,40 @@ class TestInPlacePromotion: assert not list(target.glob("00[0-9].db")) +class TestWalCheckpointing: + """Regression test: WAL files must be reclaimed between executor + phases so they don't pin disk through subsequent passes. The + 2026-05-26 production migration came within 7 GB of ENOSPC because + SQLite auto-checkpoint can't run while a reader cursor is open and + the FTS rebuild keeps a cursor open through every chunk. Manual + PRAGMA wal_checkpoint(TRUNCATE) freed 27 GB.""" + + def test_no_large_wal_after_migration( + self, source_shards: Path, audit_ndjson: Path, tmp_path: Path + ): + target = tmp_path / "tgt" + plan = HydrationPlanner().plan( + source_dir=source_shards, + target_dir=target, + target_M=4, + free_bytes=200 * 1024 ** 3, + ) + execute_plan(plan, audit_events_ndjson=audit_ndjson) + + # After completion, no .db-wal file should retain more than a + # few MB. The checkpoint(TRUNCATE) calls inside the executor + # collapse the WAL between phases; final VACUUM produces a + # zero-WAL state. + for i in range(plan.target_M): + for suffix in (".db-wal", ".db.new-wal"): + wal = target / f"{i:03d}{suffix}" + if wal.exists(): + assert wal.stat().st_size < 4 * 1024 * 1024, ( + f"WAL retained {wal.stat().st_size / 1e6:.1f} MB " + f"at {wal} — checkpoint between phases didn't fire" + ) + + class TestCrossShardForeignKeys: """Regression test: derivations.src_root can legitimately reference a surface doc that hash-routes to a different target shard. FK