Skip to content

Immutable/volatile split of LeiosDB + garbage collection V2 - #2261

Draft
geo2a wants to merge 2 commits into
leios-prototypefrom
geo2a/leios-969-leiosdb-gc-two-files-same-tables
Draft

Immutable/volatile split of LeiosDB + garbage collection V2#2261
geo2a wants to merge 2 commits into
leios-prototypefrom
geo2a/leios-969-leiosdb-gc-two-files-same-tables

Conversation

@geo2a

@geo2a geo2a commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Implements input-output-hk/ouroboros-leios#969

This PR is an alternative to #2223.

TL;DR

Implement leiosDbPromoteToImmutable and leiosDbGarbageCollect.

  • the LeiosDB is now two SQLite files — a volatile partition (<path>.vol) and an immutable partition (<path>.imm) — with the identical schema in both;
  • promotion pins the certified EB in the volatile file and hands it to a background copier thread, which appends its closure to the immutable file;
  • garbage collection is an incremental mark-and-sweep:
    • the ChainDB's GC thread marks rows;
    • a separate background sweeper thread deletes them in small, paced batches.

The EB lifecycle

The volatile ebs table gains a status column, which governs how old certified EBs move from volatile to immutable and how the unneeded ones are garbage collected:

stateDiagram-v2
    s0: 0 volatile
    s1: 1 pinned
    s2: 2 copied (evictable)
    s3: 3 marked for GC (awaiting sweeper)
    [*] --> s0: announcement inserted
    s0 --> s1: leiosDbPromoteToImmutable
    s1 --> s2: copier thread
    s0 --> s3: GC mark
    s2 --> s3: GC mark
    s3 --> s1: on late certification
    s3 --> [*]: sweeper evicts
Loading

Component walkthrough

Volatile -> Immutable promotion (sqlPromoteToImmutable)

Two steps: UPDATE ebs SET status = 1 for every announcement row of the hash (the pin — the durable record that this EB is certified and must never be evicted), then a non-blocking enqueue to the copier's TBQueue. The pin also rescues a GC-marked EB (status IN (0, 3)): a certification arriving between mark and sweep unmarks the hash.

The copier thread (startCopier)

One long-lived connection whose main database is the immutable file with the volatile file ATTACHed, plus six statements prepared once (CopierConn). Per dequeued hash:

  1. Presence probe on the immutable file — if the EB is already there, this is a duplicate delivery or a replay of a crash before the mark-as-copied; skip to that step.
  2. a completeness probe (body row count vs. closure LEFT JOIN count — a pinned EB with a truncated closure is an upstream bug, and the copier refuses to copy it rather than persist a truncation), then the three INSERT … SELECTs (announcement row, body rows, tx blobs with OR IGNORE for txs shared with earlier EBs). DEFERRED rather than IMMEDIATE on purpose: IMMEDIATE would take the write lock on the attached volatile file too for the whole copy, contending with the hot insert paths. It is safe because this thread is the immutable file's only writer, so the deferred upgrade can never hit BUSY_SNAPSHOT.
  3. Mark as copied (status 1 → 2) in the volatile file, strictly after the immutable COMMIT.

Errors are traced, the connection dropped (statements finalized first, close checked), and the thread continues with the next item.

GC mark (sqlGarbageCollect)

Called by the ChainDB's GC scheduler with the same slot it uses for the VolatileDB. It only:

  1. Self-heals: re-enqueues any stale pins (status = 1 older than the slot) to the copier — this recovers a crashed copier, a dropped queue entry, and a crash between the copy and the mark-as-copied.
  2. Runs a cheap no-op guard (an EXISTS seek on a partial index) so quiet ticks cost nothing.
  3. In one transaction: stages the txs of every EB about to be marked as orphan hints into the persistent gcTxCandidates table, then marks for GC (status = 3) every row of every hash with no pinned announcement and no announcement at or after the slot. Staging runs strictly before the mark UPDATE (the update would hide the rows from the staging SELECT); one transaction means a crash can never leave a GC-marked EB without its hints.
  4. Rings the sweeper's doorbell (an MVar ()).

The per-hash veto (NOT EXISTS a pinned or newer announcement) is what makes marking safe against pins and re-announcements: a re-announced hash's old rows simply wait until its newest announcement ages past the frontier.

GC sweep (startSweeper)

Woken by the doorbell (which starts full, so an interrupted sweep resumes on restart — the marks and hints are persistent). A sweep pass has three parts:

  • One-off GC-candidates initialisation (reinitialiseGcTxCandidates): txs referenced by no EB (left behind by the pre-mark-and-sweep GC) are staged into gcTxCandidates by keyset pagination (4096 hashes per page, no blob reads), guarded by a read-only EXISTS probe so healthy databases pay nothing.
  • EB phase: repeatedly pick up to 4 hashes all of whose rows are marked for GC — the all-marked check is re-evaluated inside the batch transaction, under the write lock, so a hash rescued by a pin between batches is never evicted — and delete their ebTxs, ebsMissingTxs and ebs rows.
  • Orphan phase: resolve hints in batches of 1024 — delete a tx only if provably unreferenced (NOT EXISTS in ebTxs; the hint is advisory, the predicate is authoritative), pop the hints either way. This phase is gated on zero GC-marked rows remaining (checked inside the transaction): hints are popped unconditionally, and a hint popped while some EB referencing that tx is marked-but-unswept would leak the tx forever. A tx still referenced when the gate is open belongs to a live EB, whose own eventual marking re-stages the hint.

After a pass that did real work, one PRAGMA wal_checkpoint(PASSIVE).

Observability

New trace events:

  • TraceLeiosDbStats --- volatile/immutable EB counts + WAL size, sampled every 10 s from an in-memory LeiosDbStats IORef.
  • TraceLeiosDbCopiedToImmutable --- the copier committed this many EBs' closures to the immutable partition.
  • TraceLeiosDbEvicted --- a sweep pass evicted this many EB announcement rows from the volatile partition.
  • TraceLeiosDbGCError --- a sweep pass failed; the connection is dropped and the pass retried.
  • TraceLeiosDbCopyQueueFull --- a promotion was dropped on a full copy queue (harmless: GC self-heal re-delivers).
  • TraceLeiosDbCopyError --- the copier failed on an EB (which stays pinned and is retried).
  • TraceLeiosDbCall --- call-trace spans around the copy, mark and sweep phases (same "kind": "Call" shape as the forge loop's, so one dashboard query covers both).

Benchmarks

New leios-gc-bench: populates a synthetic database (or runs against a copy of a production .vol.db fixture) and drives promote → copy → mark → sweep cycles, reporting per-cycle CSV (copyWaitSeconds — the wait for the background copier to catch up — markSeconds, sweepSeconds, evicted row counts). The headline behaviors it demonstrates: the mark returns in milliseconds regardless of backlog (the previous single-shot GC took multiple seconds at k=2160 fixture scale, quadratic in the backlog), and eviction throughput is bounded per transaction so the hot paths never see a long write-lock hold. leios-db-bench is only adapted to the two-path constructor.

Devnet validation

I've validated this code by running proto-devnet with k=2160 for 2 days. I specifically wanted to make sure that:

  • once we the volatile partition reaches k blocks, it stops growing and the copy-and-gc mechanism kicks in
  • the size of the volatile partition did not affect the length of the forge spans
  • the copy and GC spans stated withing reasonable time frames of several seconds at most

The following is the plot of the aggregated data which shows the duration of the forge and the new background tasks over this long devnet run:

spans-devnet-node1

@geo2a
geo2a force-pushed the geo2a/leios-969-leiosdb-gc-two-files-same-tables branch from 2c3903f to 4500c0e Compare September 2, 2026 11:52
@geo2a geo2a self-assigned this Sep 2, 2026
@geo2a
geo2a force-pushed the geo2a/leios-969-leiosdb-gc-two-files-same-tables branch 4 times, most recently from 03162cf to 772e732 Compare September 3, 2026 13:56
@geo2a geo2a added the Leios label Sep 3, 2026
@geo2a
geo2a force-pushed the geo2a/leios-969-leiosdb-gc-two-files-same-tables branch 4 times, most recently from c81a90a to da2ba6b Compare September 4, 2026 08:00
@geo2a
geo2a force-pushed the geo2a/leios-969-leiosdb-gc-two-files-same-tables branch from da2ba6b to ecfbc3d Compare September 4, 2026 09:12
@nfrisby

nfrisby commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Met with Georgy just now for 2 hours.

  • The GC/pruning code looked generally like what I was expecting 👍
  • It's got some Claude induced noise that we should remove eventually, but no obvious red flags/blockers in the parts we looked at today
  • It seems we must add a new index from tx hash to EB hash (and/or to LeiosPoint). This is an unavoidable consequence of the txs table being deduped.
    • The new index might consume a good chunk of disk.
    • Its maintenance also adds latency to inserts into and deletes from the ebTxs table. Not yet sure how painful. (see benchmarks below)
    • The entire DB would be simpler if we did not dedup txs, except the LeiosTxCache would become a little more complicated. (Our disk and disk write bandwidth would increase, but not drastically more than a strong adversary could already cause.)
    • You might think we could avoid this additional index by adding a txs.refCount column, but that column isn't necessarily initialized to 0 when inserting a new row. The row's correct initial value would either require a scan of ebTxs or... the same index this column is supposed to avoid.
    • Related: I still haven't looked into how the eb.missingTxCount column is maintained; it might also need to be accelerated by the index we're trying to avoid? One possible option: I think the LeiosFetch code could takeover responsibility of issuing Acquired* notifications, in which case missingTxCount could become dead-code... or maybe just at least a one-time cost at node-startup?
  • He has implemented the insertions into the immutable database and the deletions from the volatile database, but he has not implemented the reading from the immutable
    • At least LeiosFetch servers need to do it, so downstream peers can sync from us. (should be quite simple: if slot the slot in the received MsgLeiosBlock{,Txs}Request is old enough, try to read the given offsets from the immutable, if that succeeds, short-circuit. Otherwise, run the existing handler.)
      • Note that this requires the LeiosFetch threads to hold a "connection" that can read from both immutable and volatile LeiosDb. So maybe just add lookupImmutableBody and lookupImmutableTxs or whatever to the existing LeiosDb handle.)
      • It can be validated on the proto-devnet by stopping one node (F9 in process-compose), deleting it's database, and restarting it (F7). The syncing node will be stuck.
    • At least node initialization needs to do it when reconstructing the immutable tip ledger state by replaying immutable RBs (ie CertRBs). (even simpler than LeiosFetch server because replay logic will always read its EB from the immutable; however, it means adding a flag to resolveLeiosClosure, I'm guessing, to tell it which half of the db to read from?)
      • It can be validated by stopping and then restarting a proto-devnet node. If there's a CertRB between the node's ledger snapshot file and its imm tip, then it'll crash when replaying that CertRB.
    • Neither of those is trivial, but it's simpler than the code he's already written, at least. The work is mostly piping
    • ... ah... the LocalChainSync server also needs to be able to read from both immutable and volatile, but that maybe isn't as high-priority of a blocker.... except that db-sync needs it, I think!
  • He's shown that the sizes and latencies have the expected shape: imm grows, vol stays steady, vol access times stay steady, etc on the proto-devnet. The outliers are pretty high (~6 seconds), but that's not necessarily intolerable, at least not yet.
    • We sketched out a synthetic benchmark: load the volatile database (low, medium, worst-case) and run the new copy/mark/sweep steps to see their corresponding latency at those load levels. Also vary the number of GC'able EBs: 0, 1, 10, 100, 1000, for example (which I think you could do just by simply artificially increasing the slot number you pass into the routines?).
    • The worst-case load has no dup txs: every tx is unique to an EB.
    • That'll give us a better understanding of how bad the GC routine's latency/contention overhead could be, beyond the sanity/smoke test that's current passing on the proto-devnet (and the testnet sync he's about to run).
  • It'd be useful for that benchmark setup to also measure the time to insert a full EB under various load levels. And a useful data point would be how those durations change with and without the presence of the new idx_ebTxs_txHashBytes index.

@dnadales dnadales left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since @nfrisby gave some feedback and the state of the PR might be quite different on Monday I did not spend too much time filtering Son of Anton's comments, but they looked reasonable. HTH.

Save for these remarks, the two-staged GC process makes sense, but of course, we need to fine tune it 👍

-- re-enqueueing.
sql_gc_stale_pins :: String
sql_gc_stale_pins =
"SELECT DISTINCT ebHashBytes FROM ebs WHERE status = 1 AND ebSlot < ?1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the self-heal can see every EB that needs rescuing. This query looks for status = 1, but there seems to be a state where the pin never happened at all.

In copyToImmutableDB (Background.hs:237-245) we append the cert-RB first and pin its certified EB afterwards:

ImmutableDB.appendBlock cdbImmutableDB blk
-- ...
forM_ (certifiedEb getBI hash) $ leiosDbPromoteToImmutable cdbLeiosDb

If we crash between those two lines, don't we end up with an immutable cert-RB whose EB is still status = 0? If so, I think that EB is invisible to every recovery path we have:

  • this query only sees status = 1;
  • both leiosDbScanCompleteEbClosuresNotOlderThanSlot callers are bounded to slot >= immTipSlot (ChainDB/Impl.hs:225, NodeKernel.hs:764), and the EB's announcement slot is strictly below the tip, since it is the announcing predecessor of a now-immutable cert-RB;
  • sql_gc_markable accepts status = 0, so the next frontier past that slot marks it and the sweeper evicts both partitions' rows.

And then resolveLeiosClosure errors, both on ledger replay and on serving that block.

The window does not feel small either: the pin opens a fresh connection, runs the sql_schema_gc DDL, and then waits on BEGIN IMMEDIATE with a 1 s busy_timeout per refusal.

What about pinning before we append? Something like:

getBI <- atomically $ VolatileDB.getBlockInfo cdbVolatileDB
forM_ (certifiedEb getBI hash) $ leiosDbPromoteToImmutable cdbLeiosDb
ImmutableDB.appendBlock cdbImmutableDB blk

I guess the worry there is a leaked pin if the append then fails. But toCopy is blocks already deeper than k, so they cannot be rolled back, and sql_pin_eb is idempotent, so a crash in the new window gets repaired by the re-copy on restart. The residual cost looks like a copied EB whose cert-RB append failed, which is space rather than correctness.

pure True
-- make the EB as copied
when copied $
useStmt ccMarkAsCopied $ do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't the immutable commit and this mark in two different durability domains? appendToImmutable commits to .imm and then this UPDATE vol.ebs SET status = 2 is a separate autocommit write on .vol, and I do not see anything ordering the two files.

With synchronous = normal (line 243) a WAL commit is not fsynced, so on a power loss I think we can keep this mark and lose the immutable frames. That is the one direction the design treats as impossible: the EB then becomes evictable from .vol while being absent from .imm, and sql_imm_filter_present's "Presence is proof of a complete closure" no longer holds.

What makes me more nervous is that this looks like the likely direction rather than merely a possible one. .vol is written on every announcement, body and tx insert, so it reaches wal_autocheckpoint = 1000 constantly and fsyncs often. .imm is appended only by this thread, so its WAL can sit unsynced for a long time.

I could not find a recovery path: sql_gc_stale_pins only looks at status = 1, sql_gc_markable accepts status = 2, and initialStats only counts rows. Promotion is also forward-only, so the EB is never re-enqueued.

For what it is worth, this is a process-crash-safe / power-loss-unsafe split. A SIGKILL leaves the page cache intact and both commits survive, so the status = 1 self-heal handles that case fine.

There does seem to be a repair window we are not using: right after the power loss the .vol closure rows are still there, and they only go away once a later frontier marks and sweeps them. So maybe the cheapest fix needs no fsync at all. What about having the self-heal also re-enqueue status = 2 hashes whose closure is still present in .vol? Something like:

-- alongside sql_gc_stale_pins
SELECT DISTINCT ebHashBytes FROM ebs WHERE status = 2 AND ebSlot < ?1

and let sql_imm_has_eb short-circuit the ones that are genuinely there. Otherwise a PRAGMA wal_checkpoint(FULL) on the imm connection between the copy and this mark would do it, and since we are the only writer of that file the cost lands on a low-frequency path.

sql_decrement_missing_tx_count =
"UPDATE ebs SET missingTxCount = missingTxCount - 1\n\
\WHERE ebHashBytes IN (SELECT ebHashBytes FROM ebsMissingTxs WHERE txHashBytes = ?)\n\
\ AND status = 0\n\

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this filter go on sql_delete_missing_txs too, or come off here? The doc comment just above still says this "must be paired with sql_delete_missing_txs in the same transaction", but that statement is untouched by this PR:

DELETE FROM ebsMissingTxs WHERE txHashBytes = ?   -- no status filter

Both run in the same sqlInsertTxs transaction, and the delete is not conditional on the decrement having matched. So for a row whose status is not 0, don't we delete the hint without decrementing the counter that hint was there to drive?

I think status = 3 is reachable with a positive missingTxCount, because sql_gc_markable never reads missingTxCount: any row with status IN (0, 2) below the frontier gets marked, complete closure or not. The shape that seems to bite is a mixed hash:

  1. every row of hash H gets marked status = 3;
  2. a fresh announcement of H inserts a new row at the schema default status = 0, whose missingTxCount is still NULL;
  3. a shared tx arrives. This decrement matches only the new row, and NULL - 1 = NULL. The status = 3 row keeps its positive count;
  4. the delete removes the ebsMissingTxs rows for that tx, for both.

After that I do not think either row can ever reach 0, so sql_find_complete_ebs never matches, no AcquiredEbTxs fires, leiosAcquiredEbsRunner never calls addReprocessLeiosEb, and the cert-RB certifying H stays parked.

What worries me most is that a restart does not seem to clear it either: sql_scan_complete_ebs_since also wants missingTxCount <= 0, and the imm presence probe only helps if the EB was copied, which in this state it was not. If that is right, it is a change in behaviour against the base branch, where the row would have reached 0/-1 and the per-hash restart backstop would still have reported the hash complete.

I wonder if the safer direction is to drop AND status = 0 from this statement rather than add it to the delete. Filtering the delete leaves stale ebsMissingTxs rows that only sql_gc_missing_txs clears at sweep time, which is exactly what the mixed case blocks, and sql_init_missing_tx_count counts those rows, so a later body insert would over-count. Keeping the counter truthful for every status looks safe: each hint is deleted once, so the count converges to 0 and cannot run negative, and sql_find_complete_ebs / sql_mark_notified_ebs still gate the notification the way you added them to.

mEvictedTxs <- sweepSpan passCtx "sweepOrphanBatch" () $ \_ ->
dbWithWriteTransactionRaw tracer swDb $ do
-- don't run the sweep if any GC-marked EBs remain
blocked <- useStmt swAnyMarked $ (/= 0) <$> readSingleInt64 swAnyMarked

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this gate need to be global? sql_sweep_any_marked is SELECT EXISTS (SELECT 1 FROM ebs WHERE status = 3) with no hash and no slot, and sql_sweep_pick_marked needs every row of a hash to be at status = 3. So one hash stuck in a mixed state turns off the whole orphan-tx phase, which is the only phase that reclaims the tx bytes.

I think the mark itself can never produce a mixed hash, since its veto is per hash. But a row inserted after the mark can, and that path looks open to me:

  • tooOld compares the announcement slot to acquiredEbBodiesPrunedSlot (LeiosDemoLogic.hs:906), not the EB's age;
  • pruneOutstandingToImmTip drops the hash from ebState once its slots fall below the immutable tip and raises that same watermark, so for an already-marked hash ebStateHasBody is False and tooOld is False at any fresh slot;
  • so the offer is listed, the body is fetched, and sqlInsertEbPoint commits a new row at the schema default status = 0.

And the two frontiers differ, which is what makes the state persist: the mark frontier is the delayed GC slot, while the prune watermark is the live immutable tip. So the new row is always at or above the frontier and cannot be marked on that tick. As far as I can tell nothing is lost, since ebLoop is ungated and gcTxCandidates is persistent, so the backlog drains once a later frontier passes the new slot. But that is cdbGcDelay plus up to k blocks with no tx bytes reclaimed at all.

The part I am least comfortable with is that offers are still trusted (the TODO at LeiosDemoLogic.hs:1713-1720 says so) and the body check validates size and hash but not the slot. So one peer can re-arm this cheaply and repeatedly, and then the delay is not bounded by k any more.

I do follow why the gate exists: sql_sweep_pop_orphans pops unconditionally while sql_sweep_orphan_txs deletes conditionally, so popping a hint whose EB is marked-but-unswept would leak that tx. But couldn't we make the gate per tx instead of global? Something like popping only hints that no status = 3 row references:

DELETE FROM gcTxCandidates
WHERE txHashBytes IN (SELECT unhex(je.value) FROM json_each(?1) je)
  AND NOT EXISTS (SELECT 1 FROM ebTxs et JOIN ebs e ON e.ebHashBytes = et.ebHashBytes
                  WHERE et.txHashBytes = gcTxCandidates.txHashBytes AND e.status = 3)

I guess that would mean sql_sweep_pick_orphans needs a cursor, since today the unconditional pop is what guarantees the loop advances past hints it cannot delete.

gcRootCtx <- rootCallCtx "leiosdb-gc"

-- LeiosDB GC sweeper thread
sweepDoorbell <- newMVar ()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the doorbell start empty, and should these three threads take a registry?

Two things I noticed here. The doorbell starts full, so one sweep pass runs as soon as the handle exists, before anyone calls leiosDbGarbageCollect. I understand the intent (an interrupted sweep resumes on restart, since the marks and hints are persistent), but it also means every caller of newLeiosDBSQLite gets an unrequested pass, including the tools and every test case.

And these three forkIOs (startVolatileStatsSampler, startCopier, startSweeper) are, as far as I can tell, the only bare forkIO calls in any non-test src/ tree of ouroboros-consensus*. Everywhere else we use forkLinkedThread registry. LeiosDbHandle has no close method either, and withLeiosDb brackets only a connection, so nothing can stop them: they outlive every withLeiosDb scope and every test case.

The most visible cost is in the tests. withFreshDb sits inside ioProperty, so a suite run accumulates roughly a thousand handles and three thousand threads, each sampler waking every 10 s for the rest of the run. It looks harmless per case (the copier parks on readTBQueue, and the sweeper's doesFileExist guard turns the post-teardown wake-up into a no-op), so this is about accumulation rather than a failure.

ImmDBServer/Diffusion.hs:118 is the case that made me raise it: it already has withRegistry registry in scope and uses forkLinkedThread registry' for its own scheduler, and then calls newLeiosDBSQLite outside it.

What about threading the registry through, something like:

newLeiosDBSQLite ::
  ResourceRegistry IO -> Tracer IO TraceLeiosDb -> FilePath -> FilePath -> IO (LeiosDbHandle IO)

and using forkLinkedThread for all three, plus newEmptyMVar here so a pass only runs when the mark asks for one? If we want the resume-on-restart behaviour, we could ring the doorbell once from the first sqlGarbageCollect instead.

(/= 0) <$> readSingleInt64 gsHasWork
when hasWork $ do
(nTxsStagedAsGCCandidates, nEbsMarked) <- gcSpan gcCtx "mark" () $ \_ ->
dbWithWriteTransactionRaw tracer db $ do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the mark be batched the way the sweep is? This is one BEGIN IMMEDIATE running sql_gc_stage_marked and then sql_gc_mark, and neither has a LIMIT, while ebLoop right below does 4 hashes per transaction with a 100 ms pause.

I convinced myself the steady state is fine, and it is idx_ebs_sweepable that does it rather than the hasWork guard: the index is partial on status IN (0, 2), and sql_gc_markable's leading terms match it, and marking sets status = 3 which drops the row out of the index. So the driving set is exactly the below-frontier not-yet-marked rows, and the per-row cost is one probe on idx_ebs_ebHashBytes for the veto. That matches the "milliseconds regardless of backlog" claim in the PR description. Worth noting that a sweeper backlog specifically does not enlarge it, since status = 3 rows are outside the index.

The hasWork guard does not carry that claim, though: one row satisfies it, and the frontier moves every tick, so it only skips the empty cases.

Three cases are where I think the driving set is not small:

  1. First enable on an existing volatile file. sql_schema_gc is applied on every read-write open so pre-existing files migrate on first open (SQLite.hs:1685-1690), so on a devnet-sized file the index gets built retroactively and this first mark stages and marks the whole backlog in one transaction.
  2. A wedged copier. The live.status = 1 veto keeps every row of a pinned hash in the driving set, and the self-heal only re-enqueues, so a persistently failing copy leaves a residue that gets re-scanned in full on every tick.
  3. A frontier jump after a stall, where one tick covers many slots at once.

The consequence I care about is not the ChainDB copier: while this holds the volatile write lock, sqlInsertEbPoint, sqlInsertEbBody and sqlInsertTxs all wait on the same lock, so a long mark stalls EB and tx diffusion on the mini-protocol threads.

Could we just give the mark the same treatment as the sweep? Something like a LIMIT on sql_gc_mark, with the staging restricted to the same hashes, and a re-ring of the doorbell while a tick still has rows left. It would also let us drop the "does not do much work" caveat in the comment above, which I read as describing the intent rather than the current statements.

One thing I could not settle by reading: whether sql_gc_stage_marked's e.ebHashBytes IN (subquery) drives one seek per markable hash off ebTxs' primary key, or materialises and scans ebTxs. If it is the latter, the mark is proportional to |ebTxs| per tick regardless of how few rows are markable. Did the devnet run give you an EXPLAIN QUERY PLAN for that one?

newLeiosDBSQLite tracer (volPath dbPath) (immPath dbPath)
where
volPath :: FilePath -> FilePath
volPath = (<> ".vol")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we say something when the un-suffixed leios.db is the one that exists? A node upgrading from leios-prototype has its populated database at LEIOS_DB_PATH, which after this change is neither partition, and openRawConnection will happily create both files empty and run sql_schema into them.

I am not arguing for a real migration here. Wiping the chain DB is a fine answer for the prototype. What bothers me is how the condition surfaces: replay happens inside LedgerDB.openDB (ChainDB/Impl.hs:201-206), before any networking, so the first immutable cert-RB reaches resolveLeiosClosure and we die with an error blaming chain-sel for selecting a cert-RB whose closure is absent. There is no peer to refetch from at that point, so the message points at the wrong thing.

db-analyser already handles exactly this, and its die message is the one I would want the node to give (Run.hs:274-292).

Two details in case they are useful. NoDbMarkerAndNotEmpty does not catch it, since an upgrading node already has the marker. And a manual mv leios.db leios.db.vol is not a workaround either: the base-branch ebs has no status column, so sql_schema_gc's CREATE INDEX ... ON ebs(ebSlot) WHERE status IN (0, 2) fails with "no such column: status" on the renamed file.

What about a check in newLeiosDBSQLiteFromEnv? Something like: if neither partition exists but the un-suffixed path does, die with the reason and the two paths we expected. Also worth noting that ouroboros-consensus-cardano/README.md:88-91 still documents a single DB_PATH/leios.db, and the convert-to-split-schema.py that bench/leios-gc-bench/Main.hs:15 points at is not in the tree.

-- | Whether the volatile partition holds any unstaged GC candidates (txs no
-- EB references).
sql_has_unstaged_gc_candidates :: String
sql_has_unstaged_gc_candidates =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This predicate encodes an invariant that LeiosDbConnection's own documentation disclaims, and I wonder if we should write the invariant down rather than leave it implicit here.

Common.hs:86-93 says of leiosDbInsertEbBody:

Returns any EBs whose closure just became complete because their body landed after all their txs were already present in the DB.

So the handle documents txs-before-body as a supported ordering. And leiosDbInsertTxs (Common.hs:96-105) states no ordering precondition at all, describing txs as "the global txs table". Meanwhile this probe treats every txs row without an ebTxs row as a GC candidate, and sql_sweep_orphan_txs then deletes it.

I did convince myself we are safe today: every producer inserts the body first (forge and mempool via processLeiosBlock, and the peer path is gated on BodyAcquired), and each of those is a separate write transaction, so a crash cannot commit txs without ebTxs. The anti-join re-check under the single-writer lock covers the race as well. So this is not a bug report.

Two things make me want it stated explicitly rather than left to call order. There is no type or invariant comment enforcing it on the DB side. And Test/LeiosDemoDb.hs:550-578 inserts a full tx set with no ebTxs rows at all, which is precisely the shape this probe stages for deletion, so the test suite already encodes the opposite expectation.

Would it be worth a line on leiosDbInsertTxs in Common.hs saying that a tx with no referencing ebTxs row is treated as garbage, and correcting the leiosDbInsertEbBody sentence above so the two agree? If the tx-first ordering really is supported, then I think this probe needs a slot or age bound instead, since the consequence is asymmetric: deleting before the body insert is a wasteful refetch, but deleting after it is permanent, because missingTxCount only ever decreases.

MVar () ->
SlotNo ->
IO ()
sqlGarbageCollect tracer rootCtx volPath copyQueue sweepDoorbell gcSlot =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the mark should get the same treatment the sweeper gives itself. Every SQLite helper in here can throw LeiosDbException, and garbageCollectBlocks calls this from gcThread, which is a forkLinkedThread with no handler (Background.hs:491, forked at :121-124). So a failed mark takes the node down.

I am not questioning the policy in general: the maxBusyRetries comment states it deliberately, that nothing catches LeiosDbException and the node dies, and that is the intent. And for leiosDbPromoteToImmutable I think fail-fast is clearly right, since an unpinned EB whose cert-RB just became immutable is one the sweeper is then free to evict.

The mark looks different, though. If it fails, nothing is lost: eviction defers to the next tick and the volatile partition grows a little. And the sweeper runs the same GC work behind a catch that traces, drops the connection and retries (startSweeper, the handler at SQLite.hs:684). So the identical operation is fail-fast when the ChainDB scheduler drives it and fault-tolerant when the sweeper drives it, which seems more like an accident of where the call sits than a decision.

Would wrapping the call in garbageCollectBlocks in a trace-and-continue make sense, so both paths agree?

Separately, the comment above that call site still describes the handler as a no-op:

Driven by the same scheduled slot as the other stores; currently a no-op (see leiosDbGarbageCollect).

Same for bench/leios-db-bench/Main.hs:159 and the two InMemory.hs comments at :108 and :110, which now describe the in-memory stub rather than the interface.

( do
tmpDir <- createTempDirectory sysTmp "leios-test"
db <- newLeiosDBSQLite nullTracer (tmpDir <> "/test.db")
db <- newLeiosDBSQLite nullTracer (tmpDir <> "/test.db.vol") (tmpDir <> "/test.db.imm")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should some of this land with a test? This is the only line the test suite changes, and as far as I can tell nothing here calls leiosDbPromoteToImmutable or leiosDbGarbageCollect, so the copier, the mark and the sweeper have no coverage. InMemory.hs still stubs both as pure () (:108-111), so the handler tests exercise the no-ops rather than the new code.

leios-gc-bench is the closest thing we have, but it measures rather than asserts: it never reads a closure back after a sweep, so it would not notice an EB that was evicted while still needed.

What makes me raise it is where the correctness argument actually lives. Most of the reasoning in the PR description is about orderings and crash windows, and those are the parts a devnet run is least likely to hit. The things I would most want pinned down:

  • promote, copy, mark, sweep on one EB, then read its closure back and check it comes from .imm;
  • a mark, then a re-announcement at a fresh slot, then a sweep, and assert the rescued hash survives (this is the mixed-status case I raised on orphanTxsLoop);
  • a pin arriving between mark and sweep, and assert nothing is evicted;
  • an EB whose closure is incomplete when the mark runs, and assert AcquiredEbTxs still fires once the txs land (the sql_delete_missing_txs thread).

The first two look cheap here, since withFreshDb already gives a real SQLite pair and the handlers are on the handle. The last two need the copier and sweeper to be drivable synchronously, which today they are not, since both are forkIO loops with no way to step or await them. If the registry change I suggested on newLeiosDBSQLite happens anyway, would exposing a "run one pass and return" entry point for the sweeper be worth it, so a test does not have to race the doorbell?

Happy for this to be a follow-up PR rather than a blocker, but I would rather it be a tracked one than an implied one.

@ch1bo

ch1bo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@nfrisby Replying to your comment above:

When I read this

The entire DB would be simpler if we did not dedup txs,

I thought: that's a bad idea, or that it was a good idea that we normalized the txs into their own table. But within the same comment this

  • The worst-case load has no dup txs: every tx is unique to an EB.

made me stop and reflect. We must assume the worst case anyways and have other measures to prune the working set if it were to be too much data to retain with duplicated / denormalized txs.

So I think I agree with your viewpoint here and we can avoid quite some full table / index scans and make read access more sequential by just inlining the txs (again).

@geo2a

geo2a commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@nfrisby thanks for the notes!

but he has not implemented the reading from the immutable

Actually, the PR already implements this. My brain was too fried on Friday to remember this, sorry about that.

See specifically sqlLookupEbBody, sqlBatchRetrieveTxs, sqlLookupEbClosure and sqlScanCompleteEbPointsSince.

All changes follow the same pattern: they try reading from the block from the volatile partition and, if that fails, try reading from the immutable one instead. This is consistent with what ChainDB's getAnyBlockComponent does.

To validate, I've done a devnet experiment: stopping a node, deleting it's DB and observing that it can catch up. I've also synced a node with the Musashi testnet.

if slot the slot in the received MsgLeiosBlock{,Txs}Request is old enough, try to read the given offsets from the immutable, if that succeeds, short-circuit. Otherwise, run the existing handler.

I'm a little worried that trying to reading from the immutable partition first may expose us to the race condition caused by the fact that copying is to the immutable is delayed. I think I'd stick with the existing approach from ChainDB: read volatile, and if that has nothing, read immutable. We can introduce optimisations as a follow-up if we deem them necessary.

the LocalChainSync server also needs to be able to read from both immutable and volatile

probably this should just work as is, as it uses the same interface? Not sure, we'll need to check.

Thanks for the benchmarking plan, I'll get to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants