feat(flatdb): persist via SST ingestion (opt-in)

[AnkushinDaniil]

Issue

At x10 (~2.2 TB) the flat-DB checkpoint commits a full compacted snapshot (~32 blocks) as one RocksDB WriteBatch per column. That overflows the memtable, forces a flush, and the L0 pileup + compaction burst stalls concurrent reads for tens of seconds.

Fix

Persist via SST ingestion instead of the memtable path: build the SST files off the reader gate and ingest them (crash-atomic with a redo marker + startup sweep). Opt-in via --FlatDb.PersistViaSstIngestion (default off).

Benchmark — x10, confound-free 3× median (flag off vs on)

metric	off	on
longest persist stall	40 s	3 s
slow-task count	50	12
peak RSS	61.8 GiB	58.2 GiB
getProof p99 / p99.9	19.8 / 33.9 ms	18.8 / 31.0 ms

The stall is real on master (44 s, 54 slow tasks/run); SST cuts it to 3 s (−93%), every round, at RSS −6% and read latency flat-to-better. x1 mainnet (single run): RSS 45.7 → 40.4 GiB, getProof flat.

[claude]

Claude finished @AnkushinDaniil's task in 5m 24s —— View job

---

Review — SST-ingestion persist path (PR #12401)

• Read changed files
• Analyze correctness / edge cases / concurrency
• Analyze native RocksDB bindings & resource management
• Review tests
• Post review

This is a well-structured, clearly-motivated change with strong benchmark evidence and thoughtful failure handling. It's correctly marked draft and the flag is default-off, so nothing here is production-live. My comments are what I'd want resolved (or explicitly acknowledged) before the flag is ever flipped on. Inline comments posted on specific lines.

Findings

Medium

• Cross-column read atomicity is lost during a persist — not only crash-atomicity. The current path commits all six columns in one WriteBatch, so a concurrent snapshot sees the persist atomically. The ingest path ingests each column's SST independently and advances currentState afterwards, so a reader that pins a snapshot mid-persist can see Account at block N while Storage is still at N-1 (torn base state) with currentState == N-1. This affects live RPC reads during persist — the exact workload this PR targets — even with no crash. The planned multi-CF atomic ingest closes this too, but the PR body only frames the gap as crash durability. (RocksDbPersistence.cs)
• Test-coverage gap on the correctness-critical failure path. SstIngestionTests covers the happy paths and dedup, but not the rollback invariant that the whole failure-handling design rests on: an ingest failure must not advance currentState (so recovery re-executes from the prior pointer). Per AGENTS.md, a regression test for that path (inject an ingest/finish failure, assert pointer unchanged + staged .sst cleaned up) should land with the feature. No concurrent-reader consistency test either.

Low

• Fallback new ColumnFamilyOptions() leaks a native handle and silently drops the CF's real options (ColumnDb.cs:283) — defeats commit a1e4a6e when hit; assert non-null or using it.
• Backpressure silently gives up after ~30 s with no logging (ColumnDb.cs:318) — the L0 pile-up it guards against becomes undiagnosable; add a log + named constants.
• &_slabs[e.Slab][e.Offset] throws on a zero-length reserve at a slab boundary (ColumnDb.cs:293) — unreachable for 32-byte state keys, but the batch is a general public IWriteBatch.
• Static pools retain up to ~1 GB (slabs) + ~1.3 GB (entries) permanently (ColumnDb.cs:136) — document the worst-case retained footprint; consider a smaller entry-pool bucket count.

Things that look correct

• Per-chunk dedup (stable sort by KeyPrefix → bytewise → Seq, keep last of each run) matches RocksDB's default bytewise comparator — verified no custom comparator is set on any Rocks column, so the manual sort order is safe. Cross-chunk dedup via AllowGlobalSeqno + ingest order is sound. Seq resetting per chunk is fine because ordering is only needed within a chunk.
• Key/value stored contiguously per entry; data + KeyLen pointer arithmetic is correct; delete tombstones (ValLen < 0) handled.
• Failure handling disposes all six batches + the snapshot in try/finally, surfaces the first failure via ExceptionDispatchInfo, and deletes staged .sst on failure.
• is ISstIngestible gate + MemDb fallback is correct and tested.

Verdict

Not mergeable yet — which matches the author's own draft status. The blocking items are the two the PR already lists (multi-CF atomic ingest + startup orphan sweep); I'd add the live-reader torn-state dimension and the failure-path regression test to that list. I did not build/run the test suite in this review.

[AnkushinDaniil]

@claude re-review

[claude]

Claude finished @AnkushinDaniil's task in 1m 45s —— View job

---

Re-review — SST-ingestion persist path (PR #12401)

• Read current state of changed files
• Verify the four Low findings were addressed by recent commits
• Re-assess the two Medium findings
• Post re-review

The five review commits since the last pass resolve every Low finding and explicitly acknowledge one of the two Mediums in the PR body. Nice, targeted fixes. Details below.

Low findings — all resolved ✅

Finding	Status	Where
Fallback new ColumnFamilyOptions() leaks a native handle / drops real CF options	Fixed — now ?? throw new InvalidOperationException(...), fails loud, no leak (GetColumnFamilyOptions is a dict lookup returning DB-owned options)	ColumnDb.cs:289-290 (324e9d2)
Backpressure silently gives up after ~30 s, no logging	Fixed — magic numbers hoisted to L0DrainMaxPolls/L0DrainPollMs/MaxL0FilesBeforeThrottle, warns on timeout with column + duration	ColumnDb.cs:136-137, 335-336 (5fe9204)
&_slabs[e.Slab][e.Offset] throws on zero-length reserve at a slab boundary	Fixed — pins the whole slab via MemoryMarshal.GetArrayDataReference + offset, so a one-past-end pointer with KeyLen==0 no longer bounds-checks; covered by Ingest_zero_length_write_at_exact_slab_boundary_round_trips	ColumnDb.cs:300 (713cd48)
Static pools retain ~1 GB slabs + ~1.3 GB entries permanently	Addressed — entry-pool bucket count trimmed 8→6 (matches the six column batches of one in-flight persist) and worst-case retention documented inline	ColumnDb.cs:140-143 (c824dc3)

Medium findings

1. Cross-column read atomicity during a persist — now acknowledged. ✅
The PR body's "Live torn reads" bullet now names this explicitly as a merge blocker distinct from crash-atomicity, with the same fix (multi-CF rocksdb_ingest_external_files) closing both windows. The gap itself still exists in code — each column ingests at its own global seqno while currentState advances afterward, so a reader pinning db.CreateSnapshot() mid-persist can still observe a torn base state — but it's correctly documented and gated behind the default-off flag + draft status. Acknowledged with rationale.

2. Failure-path rollback regression test — still open. ⚠️
SstIngestionTests gained good coverage (round-trip + pointer advance, self-destruct/recreate dedup, cross-128-MiB-chunk dedup, zero-length slab-boundary, MemDb fallback), but the invariant the whole failure design rests on is still untested: an ingest/finish failure must not advance currentState (recovery re-executes from the prior pointer).

The invariant is correctly implemented — in CreateIngestWriteBatch, firstFailure?.Throw() (RocksDbPersistence.cs:107) runs before the SetCurrentState metadata batch (:111), so a failed ingest short-circuits the pointer write and staged .sst is deleted in the catch at ColumnDb.cs:316-320. It just has no regression test. Per AGENTS.md, a test that injects an ingest/finish failure and asserts (a) pointer unchanged and (b) sst_ingest/ cleaned up should land with the feature. The PR body lists the concurrent-reader consistency test as blocker (3) but not this one — worth adding to the blocker list.

Add failure-path rollback test →

Verdict

Still not mergeable — matching the author's own draft status. All four Low findings are resolved and the live-torn-read atomicity dimension is now explicitly documented as a blocker. The remaining items are the author's listed blockers (multi-CF atomic ingest + startup orphan sweep + concurrent-reader test), plus I'd still add the failure-path rollback regression test to that list. Nothing here regresses existing behavior — the flag stays default-off.

I did not build or run the test suite in this review.
· branch daniil/split-sst-ingestion

[AnkushinDaniil]

@claude re-review

[claude]

Claude finished @AnkushinDaniil's task in 1m 51s —— View job

---

Re-review — SST-ingestion persist path (PR #12401)

• Read new commit since last review (4c534ec)
• Verify failure-path rollback regression test addresses prior Medium Feature/networking #2
• Re-assess remaining open blockers
• Post re-review

One new commit since the last pass (4c534ec), and it lands exactly the test I'd flagged as the outstanding gap. Details below.

Prior Medium #2 — failure-path rollback regression test — now resolved ✅

Failed_ingest_leaves_pointer_and_staging_dir_untouched (SstIngestionTests.cs:178) exercises the real failure path and asserts both halves of the invariant the failure design rests on:

• Pointer not advanced. A _testIngestFailureHook on the Account column throws IOException from inside FlushChunk (ColumnDb.cs:315), before IngestExternalFiles. Account's batch is disposed first (RocksDbPersistence.cs:100), so the exception is captured as firstFailure; the remaining batches still dispose, then firstFailure?.Throw() (:107) re-throws before the SetCurrentState metadata batch (:111). The test confirms reader.CurrentState == s1 — the persist to s2 is not committed, so recovery re-executes from the prior pointer. Correct.
• No orphaned staged .sst. Account's SST is deleted in the catch (ColumnDb.cs:320); the other columns that did dispose successfully ingest with SetMoveFiles(true) (ColumnDb.cs:155), so RocksDB moves their files out of sst_ingest/. The test asserts sst_ingest/*.sst is empty. Correct.

This is the right way to test it: the hook injects the failure at the exact native-ingest boundary, so the assertion covers the real ColumnDb catch/cleanup rather than a mock. It maps directly onto the author's blocker item (4), now marked done in the PR body.

Note (not blocking): the test seam is a production-code field (internal Action? _testIngestFailureHook, ColumnDb.cs), invoked via ?.Invoke() once per chunk flush — negligible overhead and gated to test assemblies via InternalsVisibleTo. Given it's the only way to drive the real cleanup path from a test, the tradeoff is reasonable; worth a one-line comment on the field noting it's a test-only injection point so a future reader doesn't mistake it for a production hook.

Status of the four earlier Low findings — all still resolved ✅

No regressions from the earlier fix commits (fail-loud CF options, named L0-drain constants + warn, whole-slab pin for zero-length entries, trimmed entry-pool buckets + documented retention). Nothing in 4c534ec touches them.

Remaining open blockers (unchanged, author's own list)

1. Multi-CF atomic ingest (rocksdb_ingest_external_files across CFs) — closes both the crash-atomicity window and the live cross-column torn-read window. Open.
2. Startup sweep for orphaned staged .sst — open.
3. Concurrent-reader snapshot-consistency test across a persist — open.

The live torn-read dimension is now correctly documented in the PR body, and this failure-path test closes item (4).

Verdict

Still not mergeable — matching the author's own draft status. The remaining gaps are items (1)–(3) above, all of which the author already lists as merge blockers. The failure-path rollback test I'd asked for is now in place and correct. The flag stays default-off and nothing here regresses existing behavior.

I did not build or run the test suite in this review.
· branch daniil/split-sst-ingestion

[AnkushinDaniil]

@claude re-review

[claude]

Claude finished @AnkushinDaniil's task in 3m 51s —— View job

---

Re-review — SST-ingestion persist path (PR #12401)

• Read new commit f50cf79 (redo marker, reader gate, startup sweep)
• Verify blocker (1) crash-atomicity / multi-CF atomic-commit substitute
• Verify blocker (2) startup orphan/roll-forward sweep
• Verify blocker (3) concurrent-reader consistency test
• Re-assess live cross-column torn-read window
• Post re-review

One new commit since the last pass (f50cf79), and it closes all three remaining blockers from the author's own list plus the live torn-read dimension. This is a clean, well-reasoned implementation. Details below.

Blocker (1) — crash-atomicity — resolved ✅

Rather than the multi-CF rocksdb_ingest_external_files (never wrapped in the C API), the commit makes the six single-CF ingests recoverable via a WAL-synced redo marker, which is a sound substitute:

• SetIngestMarker writes {target StateId, staged file names} to Metadata and db.Flush(onlyWal:true) before the first ingest (RocksDbPersistence.cs:138-146).
• The currentState advance and ClearIngestMarker share one WriteBatch (:157-162), so the pointer is never durable while the marker still exists, and vice-versa. On crash the DB is either fully at N‑1 (no marker) or roll‑forward‑able to N.
• RollForwardPendingIngest (:238) re-ingests only files that still exist — move-ingest (SetMoveFiles(true)) consumes sources on success, so a re-ingest of an already-consumed file is skipped (!File.Exists → continue), and a rare double-ingest of identical k/v is content-idempotent. Then it completes the pointer write and clears the marker.
• Tested: Crash_between_column_ingests_rolls_forward_on_reopen (Account ingested, Storage staged, no pointer) and Crash_after_all_ingests_completes_pointer_on_reopen (all ingested, pointer missing) — both reopen to s2 with all columns consistent, marker cleared, staging dir empty.

Blocker (2) — startup orphan sweep — resolved ✅

RecoverInterruptedIngest (:214) runs at construction, flag-independent (field initializer, before _useSstIngestion is read), so a flag-off restart still finishes or cleans a prior flag-on run — matching the PR body. A pending marker rolls forward (logged); otherwise leftover sst_ingest/* files are deleted (logged). No-op when the dir is absent. Tested: Startup_sweep_deletes_orphaned_staged_files_without_marker — orphans gone, pointer untouched.

Blocker (3) + live torn-read window — resolved ✅

A ReaderWriterLockSlim (_ingestGate) closes the cross-column torn-read window I flagged in the first review:

• CreateGatedSnapshot (:74) takes the read side around db.CreateSnapshot() (only when the flag is on).
• CommitIngest holds the write side across all six ingests and the pointer write (:147-165), so a snapshot is never pinned mid-commit — a reader sees either all columns at N‑1 (pointer N‑1) or all at N (pointer N). RocksDB whole-DB snapshots are single-seqno across CFs, so a snapshot taken outside that window is cross-column consistent.
• The L0 headroom throttle deliberately runs outside the gate (:167-169), so reader snapshot creation is never stalled behind compaction — the stated design goal.
• Tested: Concurrent_readers_never_observe_torn_cross_column_state — 2 reader threads pin snapshots in a tight loop while 25 persists run, asserting all six columns match the pinned currentState.BlockNumber; assertion failures surface through Task.WaitAll.

Earlier findings

All four earlier Low findings and the failure-path rollback test (prior Medium #2) remain resolved — nothing in f50cf79 regresses them. The _testIngestFailureHook now fires at the ingest boundary (ColumnDb.cs:145) and Failed_ingest_leaves_pointer_and_staging_dir_untouched still exercises the real RollbackFailedIngest path.

Minor nits (non-blocking, Low)

• Empty catch { } in DeleteStagedFiles (ColumnDb.cs:365) and the FlushChunk cleanup (:341) swallow delete failures silently — the repo robustness rule asks for at least a log line. Best-effort cleanup with the startup sweep as backstop, so not a correctness issue; a IsDebug line would keep a stuck-delete regime diagnosable. (inline comment posted)
• _ingestGate (ReaderWriterLockSlim) is never disposed — RocksDbPersistence/IPersistence aren't IDisposable. Process-lifetime singleton, RWLS holds no eager OS handle, so negligible; noting for completeness.
• StartSstIngestBatch() disposed without SealToStagedFiles() silently drops buffered writes (Dispose no longer flushes). It's the documented contract and the only production caller (CommitIngest) always seals+ingests, so this is by design — just a footgun on the public ISstIngestible seam.

Verdict

All previously-listed merge blockers are now addressed (crash-atomicity via redo marker + roll-forward, startup sweep, concurrent-reader consistency), and the live cross-column torn-read window is closed by the reader gate. No Critical/High/Medium items remain open. The flag stays default-off and nothing here regresses existing behavior; the only outstanding items are the Low nits above.

I did not build or run the test suite in this review.
· branch daniil/split-sst-ingestion

[claude]

Claude finished @AnkushinDaniil's task in 5m 16s —— View job

---

Re-review — SST-ingestion persist path (PR #12401)

• Read new commit f50cf79 (redo marker + reader gate + startup sweep)
• Verify multi-CF atomic ingest / crash-atomicity (blocker 1)
• Verify startup orphan sweep + roll-forward (blocker 2)
• Verify snapshot-gate reader consistency + new test (blocker 3)
• Re-assess prior findings for regressions
• Post re-review

The redo-marker commit closes the three blockers from prior rounds — but it introduces one new High correctness gap on the in-process failure path that the redo-marker design otherwise handles correctly for the crash case.

Prior blockers (1)–(3) — now addressed ✅

• Crash-atomicity / cross-column torn reads. The ReaderWriterLockSlim gate (RocksDbPersistence.cs:74-87, 160-177) takes the read side to create a reader snapshot and holds the write side across all six ingests + the pointer write, so a snapshot is never pinned mid-commit. The L0 throttle is correctly kept outside the gate. The redo marker (BasePersistence.SetIngestMarker) is WAL-synced before the first ingest and cleared atomically with the pointer advance — a process death anywhere rolls forward on reopen. Sound.
• Startup sweep + roll-forward. RecoverInterruptedIngest rolls a pending marker forward (skipping already-moved files via !File.Exists) then deletes orphan staged files; runs regardless of the flag. Per-column ingest is atomic, so "remaining files" are exactly the un-ingested columns — the ordering/idempotency reasoning holds.
• Concurrent-reader test. Concurrent_readers_never_observe_torn_cross_column_state pins snapshots across 25 persists over all six columns and asserts every column matches the pointer. Plus both crash-window roll-forward tests and the orphan-sweep test. Good coverage.

New finding

High — failure-path rollback leaves a persisted torn base once any column has been ingested. (inline)
CommitIngest ingests the six columns sequentially. If column k > 1 throws, columns 1..k-1 are already live at state N (their staged files were MoveFiles-consumed) while the pointer is still N-1. RollbackFailedIngest then clears the marker and deletes the remaining staged files — which (a) cannot un-ingest the earlier columns and (b) destroys the only means for RecoverInterruptedIngest to complete the commit. On reopen there's no marker → the torn base persists → re-executing block N reads a corrupted pre-state → state-root mismatch → block rejected.

This contradicts the PR body's stated guarantee ("the pointer is not advanced and recovery re-executes from the previous pointer"). Rollback is only safe before the first successful ingest; after that the failure path must become roll-forward-only (keep the marker + remaining files, rethrow, let startup finish).

The existing Failed_ingest_leaves_pointer_and_staging_dir_untouched test does not catch this: the injection hook is on the first column (Account) and fires before the native ingest (ColumnDb.cs:147), so no partial data is ever produced. A test injecting on the second column and asserting the first column shows no state-N data after reopen would surface it.

Earlier Low findings — all still resolved ✅

Fail-loud CF options, named L0-drain constants + timeout warn, whole-slab pin for zero-length entries, trimmed entry-pool buckets + documented retention — no regressions in f50cf79.

Minor (non-blocking, unchanged from last round): _testIngestFailureHook is a production-code field that could use a one-line "test-only injection point" comment.

Verdict

Not mergeable — one unacknowledged High correctness gap on the failure-path rollback (flag stays default-off + draft, so nothing is production-live, but it undermines a core stated guarantee and should be fixed before the flag is flipped on). The three prior blockers are genuinely closed. I did not build or run the test suite in this review.
· branch daniil/split-sst-ingestion

[LukaszRozmej]

Deep review of the SST-ingestion persist path. The happy path and the true-crash recovery look carefully thought through; the comments below are about the failure/recovery edges and a few cleanups. #1-#3 are the ones I'd treat as blocking. Generated with Claude Code.

[AnkushinDaniil]

@claude re-review

[claude]

Claude finished @AnkushinDaniil's task in 2m 47s —— View job

---

Re-review — SST-ingestion persist path (PR #12401)

• Read new commit 53a1334 (roll-forward-only failure path)
• Verify prior High finding (torn base on mid-loop ingest failure) is resolved
• Verify regression test for failure-after-first-column
• Re-assess remaining open items
• Post re-review

One new commit since the last pass (53a1334). It closes the High finding I raised — cleanly, and with a regression test that reproduces the exact gap. Details below.

Prior High — mid-loop ingest failure left an unrecoverable torn base — resolved ✅

CommitIngest now tracks how many columns have been ingested and branches the failure path on it (RocksDbPersistence.cs:143, 164-168, 183-197):

• Failure before any ingest (columnsIngested == 0) → RollbackFailedIngest clears the marker (WAL-synced) then deletes staged files. Nothing is live at N, so this is safe — unchanged behavior.
• Failure after ≥1 ingest (columnsIngested > 0) → the marker and the remaining staged files are kept, an error is logged, and the exception is rethrown. The already-live columns can't be un-ingested, so the commit is now roll-forward-only: a retried persist (regenerates all columns, overwrites the marker) or RollForwardPendingIngest on the next open re-ingests only the files that still exist (move-ingest consumed the rest via !File.Exists → continue) and advances the pointer. The pointer never advances to N while a column lacks N. Correct.

Regression test is on-point. Failed_ingest_after_first_column_keeps_marker_and_rolls_forward_on_reopen (SstIngestionTests.cs:243) injects the fault on Storage (second in the loop) so Account ingests first (columnsIngested == 1), then asserts: marker present + target s2 + staging dir non-empty after the throw, and after Reopen() — currentState == s2, all six columns readable at N, marker cleared, staging dir empty. This is exactly the injection point (non-first column) the prior test missed. Good.

The _testIngestFailureHook also picked up the "test-only injection point" comment I'd noted (ColumnDb.cs:29).

Prior blockers (1)–(3) + Low findings — still resolved ✅

Crash-atomicity redo marker, startup sweep + roll-forward, reader snapshot gate, and the four earlier Low fixes — no regressions in 53a1334.

Still open (from @LukaszRozmej's review, posted after my last pass)

Medium — backpressure runs outside the commit try/catch, so a post-commit throttle failure surfaces a committed persist as a thrown persist. (RocksDbPersistence.cs:199-201)
WaitForIngestCompactionHeadroom runs after the try/catch that durably advances the pointer + clears the marker. If GetColumnDb/GetProperty throws here (e.g. a column disposed during shutdown), the exception propagates out of CommitIngest → the write-batch Dispose() → the caller, which then sees a durably-committed block N as a failed persist. The DB itself stays consistent at N (marker cleared), but a caller that retries the persist of block N will hit the currentState != from guard (:291) and throw. Backpressure is best-effort flow control — it should swallow + log, not be able to fail a committed persist. Not addressed or acknowledged. Fix this →

Low (non-blocking), also from @LukaszRozmej — unchanged since that review:

• Empty catch { } in DeleteStagedFiles (ColumnDb.cs:366) and the FlushChunk cleanup (:342) swallow File.Delete failures silently — robustness.md asks for at least a log line. Startup sweep is a backstop, so not a correctness issue.
• unsafe block in FlushChunk (:326-331) needs a comment asserting e.Offset + e.KeyLen + e.ValLen stays within the pinned slab, per robustness.md.
• IngestColumns[] (:12), the six explicit Ingest(...) calls (:93-98), and the batches[] literal (:127) must be kept in lockstep with no compile-time link — consider driving all three from the one array.
• new EntryComparer(this) allocated per FlushChunk (:308) — stateless apart from this; cache it in a field to drop one alloc per 128 MiB chunk on the persist hot path.

RecoverInterruptedIngest running flag-independently at the field initializer (:22) is intentional and documented in the PR body (a flag-off restart must still finish/clean a prior flag-on run), so I read that as acknowledged-with-rationale rather than open.

Verdict

The prior High is genuinely fixed — the failure path is now roll-forward-only once any column is live, with a targeted regression test. The remaining open item is @LukaszRozmej's Medium on backpressure being able to fail a committed persist, plus the Low nits above. Flag stays default-off and nothing here regresses existing behavior. I did not build or run the test suite in this review.
· branch daniil/split-sst-ingestion

[AnkushinDaniil]

@claude The flag-off reopen regression test is now on head f39ef15d9c. Please re-review the current head and update the review verdict if no issues remain.

[claude]

Claude finished @AnkushinDaniil's task in 4m 43s —— View job

---

Re-review — SST-ingestion persist path (PR #12401) @ f39ef15

• Read commits since last review (2df8d5f, d0a5fab, f39ef15)
• Verify the flag-off reopen roll-forward test
• Verify the slab-reference pooling refactor
• Re-verify the inline-completion / crash-safety invariants at head
• Re-assess prior findings for regressions
• Post verdict

Three commits since my last pass. The new test lands exactly the coverage gap it claims, the pooling refactor is clean, and I re-verified the crash/torn-state invariants against the current file contents rather than the diff alone.

f39ef15 — flag-off reopen roll-forward test — correct ✅

Crash_between_column_ingests_rolls_forward_on_reopen is now [TestCase(true)]/[TestCase(false)], with Reopen(bool persistViaSstIngestion = true) threading the flag into the reconstructed RocksDbPersistence. The false case is the one that matters: it simulates Account-ingested / Storage-staged / pointer-not-advanced, restarts with PersistViaSstIngestion = false (the default), and still asserts CurrentState == s2, both columns readable, marker cleared, staging dir empty.

That is a genuine guard on the design decision at RocksDbPersistence.cs:47-52 — re-gating RecoverInterruptedIngest on the flag would fail this case, which is precisely @flcl42's "recovery depends on the next launch retaining the opt-in flag" finding. Parameterizing the existing test rather than copy-pasting a second one also matches the repo's AGENTS.md test guidance.

Minor: the flag-off arm exercises recovery + reads but not a subsequent WriteBatch-path persist on top of the rolled-forward pointer. Not worth another case — CreateWriteBatch's currentState != from guard already covers the pointer contract.

d0a5fab — pooled slab-reference list — correct ✅

List<byte[]> → ArrayPoolList<byte[]> on a dedicated ArrayPool<byte[]>.Create(1024, 6) (ColumnDb.cs:178, 183). I checked the edges:

• No stale-reference retention. ArrayPoolListCore.Dispose calls ClearToCount before pool.Return, so the pooled byte[][] doesn't pin slabs (or the oversized dedicated buffers from Reserve) after the batch ends. It's also _disposed-guarded, so the double-dispose path is a no-op.
• Over-capacity growth is safe. If the list ever grew past the pool's 1024 maxArrayLength, ConfigurableArrayPool rents unpooled and Return no-ops on the out-of-range bucket — degradation, not a throw. In practice the 128 MiB chunk cap bounds live slabs at ~128, well under it.
• Clear()/Reserve() semantics unchanged. RemoveAt still drops only non-SlabSize (dedicated) entries, so after Clear every remaining element is a pooled slab and _slabIndex = 0 is valid; the skip-loop still handles dedicated slabs appended mid-chunk.
• Retained footprint of the new pool is 6 × 1024 × 8 B ≈ 48 KB — noise next to the documented slab/entry ceilings.

2df8d5f — re-verified at head ✅

Re-read rather than re-diffed, since this is the load-bearing commit:

• catch when (columnsIngested > 0) completes the commit inline while still holding _ingestGate's write lock (:195-207), so no snapshot can be pinned over the torn base; the bare throw; when the marker is missing correctly preserves the original exception.
• The marker is WAL-synced at :181 before the first ingest, so the inline ReadIngestMarker at :204 always sees it.
• A failure of the inline roll-forward falls to the outer catch, which keeps marker + staged files and logs at Error (:237-240) — roll-forward-only, as documented.
• Backpressure is post-commit, best-effort, and bounded by one shared 30 s CancellationTokenSource across all six columns (:252-259), with WaitForIngestCompactionHeadroom checking the token each poll (ColumnDb.cs:157).

One subtlety worth knowing is load-bearing (verified safe, no change requested): the constructor now runs ResolveSlotEncoding (:44) before RecoverInterruptedIngest (:52) — the reverse of an earlier revision. That's safe only because the null => slotStore.FirstKey is null heuristic in ResolveSlotEncoding is unreachable after a crashed mid-ingest: RecordLayoutOnFirstBatch → SetLayout writes the slot-encoding version on the marker batch (:178-179), which is WAL-synced before any storage SST goes live. Were that record ever moved off the marker batch, a fresh DB crashing mid-ingest would reopen with storage rows present, resolve to legacy raw, and misread every slot. Fine as written; just fragile if that call site moves.

Everything from prior rounds — still resolved ✅

Crash-atomicity redo marker, startup orphan sweep with guarded/logged deletes, reader snapshot gate + concurrent-reader test, roll-forward-only failure path, best-effort post-commit backpressure, the four original Low fixes (fail-loud CF options, named L0-drain constants + warn, whole-slab pin, trimmed entry-pool buckets), the unsafe safety-invariant comment (ColumnDb.cs:338-340), the cached EntryComparer, and the single-array-driven ingest column list. No regressions from these three commits.

Low / non-blocking

• Redundant WAL fsync on the inline-completion path. RollForwardPendingIngest already flushes (:351), then CommitIngest flushes again at :226. Harmless, and only on a rare failure path.
• _ingestGate (ReaderWriterLockSlim) is still never disposed — process-lifetime singleton, no eager OS handle. Noted for completeness only.
• ISstIngestWriteBatch.Dispose releasing buffers without deleting staged files remains a footgun on the public seam, but it's the documented contract the keep-for-roll-forward path depends on, backstopped by the startup sweep.

Verdict

Mergeable. No Critical / High / Medium findings. Every blocker from prior rounds — including all three of @flcl42's — is closed with a targeted regression test, and this commit adds the last missing one (flag-off reopen). The flag stays default-off, so nothing changes for existing nodes.

I did not build or run the test suite (dotnet build requires approval in this environment), so the 14 SstIngestionTests cases are reviewed as source, not executed.
· branch daniil/split-sst-ingestion