Follow up fast-maintenance chain review debt after merge

[snissn]

Summary

Post-merge follow-up issue for the fast-maintenance chain.

The main correctness chain has landed. Review-debt cleanup and CI-runtime follow-ups have also landed. This issue now tracks the remaining runtime maintenance and RSS work.

The work has shifted from small, stacked fixes to a larger maintenance-design effort. The current best merged baseline is #767.

Completed

Completed by #763:

• Remove dead duplicate fallback == nil checks in TreeDB/caching/leafref_live_ids.go.
• Disable the real background generation loop in direct scheduler tests instead of racing explicit maybeRunVlogGenerationMaintenance(...) calls.
• Revisit schedulerTestWait minimum/default values enough to raise the floor for slower CI.
• Update .github/scripts/summarize_go_test_json.py to include package-level skip events.

Completed by #764:

• Review waitForRetainedValueLogPrune() inside maybeRunVlogGenerationMaintenance().
• Keep the current serialization with waitForRetainedValueLogPrune(); no behavior change needed.

Completed by #765:

• Preserve / handle snap.Close() errors instead of suppressing them.
• Simplify / reduce the rewrite-plan test hook registry production surface.
• Revisit cached rewrite live-bytes cache-hit behavior.

Completed by #766:

• Reduce TreeDB/caching CI runtime by cutting wall-clock-heavy maintenance/reopen test cost.

Completed by #767:

• Add resumable maintenance queue / checkpoint-triggered rewrite+GC entry for DisableWAL.
• Improve runtime debt behavior enough to get a real partial win over the pre-#767 baseline.

Completed by #784 as a narrow assertion-driven fix:

• Stop idle split-vlog lane checkpoint rotation from churning untouched lanes.
• Materially reduce pathological l1/l2 tiny-file explosion.

Current Best Merged Baseline

Treat merged #767 as the current baseline for future work.

Representative best merged #767 runs:

case	profile	duration_s	max_rss_kb	max_rss_gib	end_app_bytes	end_app_gib
treedb_fast	fast	250	20737692	19.31	9506969532	8.85
treedb_wal_on_fast	wal_on_fast	293	18804252	17.51	5314266168	4.95

Run homes:

• fast: /home/mikers/.celestia-app-mainnet-treedb-20260309214001
• wal_on_fast: /home/mikers/.celestia-app-mainnet-treedb-20260309214518

This is the baseline to compare against unless a newer merged change supersedes it.

Runtime Follow-Up: Celestia Evidence And Targets

The main correctness state is good enough to separate merge from perf work:

• Current merged line completes end-to-end run_celestia on both:
  • treedb fast
  • treedb wal_on_fast
• The remaining work is now perf / maintenance debt, not the earlier correctness blocker.

Earlier head Celestia runs (pre-#767 context)

case	profile	duration_s	max_rss_kb	max_rss_gib	end_app_bytes	end_app_gib	du_bytes	du_gib
treedb_fast	fast	305	20955128	19.51	14942028077	13.92	17204622099	16.02
treedb_wal_on_fast	wal_on_fast	321	17424392	16.23	11253062646	10.48	11253062646	10.48

Run homes:

• fast: /home/mikers/.celestia-app-mainnet-treedb-20260309141847
• wal_on_fast: /home/mikers/.celestia-app-mainnet-treedb-20260309141303

Manual maintenance on those earlier finished runs

Using a freshly built treemap from the head line, the following was run on each finished application.db:

1. treemap vlog-gc <db> -rw
2. treemap vlog-rewrite <db> -rw

Results:

case	before_du	after_gc_du	after_rewrite_du	total_reclaimed
treedb_fast	17204622099	7336856119	2468094302	14736527797
treedb_wal_on_fast	11253062646	5209791027	2576638928	8676423718

Approximate GiB:

• fast: 16.02 GiB -> 6.83 GiB -> 2.30 GiB
• wal_on_fast: 10.48 GiB -> 4.85 GiB -> 2.40 GiB

Interpretation:

• Both profiles converge to roughly the same final floor after maintenance: about 2.3-2.4 GiB.
• That means the large runtime disk gap is primarily maintenance debt / stale vlog accumulation, not irreducible live data.
• This remains the strongest signal for the next perf sprint.

What Has Been Tried Since #767

These are the important post-#767 experiments. Future agents should not rediscover them blindly.

Rejected small-tuning follow-ups

Rejected because they preserved correctness but regressed wall time, RSS, or runtime debt:

• #774: persisted GC queue follow-up
• #776: byte-budgeted rewrite queue phase-1 variant
• #780: checkpoint-kick GC gating change
• several smaller scheduler/cadence/chunk-size tweaks around rewrite refill cadence, maxSegments=2, MinSegmentAge, and GC/rewrite gating

Common pattern:

• correctness stayed good
• maintained floor stayed reasonable
• runtime accumulation, wall time, or RSS got worse

Conclusion:

• the remaining problem is not another small scheduler constant
• the remaining problem is algorithmic

#782 / #783 style phase-1 scaffolding

These branches produced useful architectural scaffolding and tests, but did not produce a keepable runtime win. They should be treated as design exploration, not as the current baseline.

Current Assertion-Driven Smell Investigation

The next active focus became the pathological directory shape, especially unreasonable l0 accumulation.

Pathological run shape that triggered this direction

Observed on:

• /home/mikers/.celestia-app-mainnet-treedb-20260310092036/data/application.db/maindb/wal

Main smell:

• l0 total around 16.84 GB
• hundreds of l0 segments
• hundreds of tiny l1 and l2 segments
• one l255

This is now considered a correctness-style runtime assertion problem, not just a tuning smell.

Important finding from the lane-churn fix

#784 proved one real contributor:

• idle checkpoint rotation of untouched split-vlog lanes

Comparison showed:

• l1 files: 596 -> 1
• l2 files: 596 -> 1
• tiny <1MiB files on l1: 595 -> 0
• tiny <1MiB files on l2: 595 -> 0

So:

• lane churn on l1/l2 was a real bug and was materially reduced
• but the main remaining smell is still l0

Current main smell

Even after the #784 lane-churn fix, the remaining unreasonable condition is:

• l0 still extremely large
• representative observed shape:
  • 357 l0 segments
  • about 16.84 GB in l0

This is the next main target.

Strong Current Inferences

These are the main conclusions a future agent should start from instead of rediscovering:

1. The earlier Celestia correctness blocker was real and is fixed on the merged line.

• The decisive fix was flushing unsynced fast importer value-log batches before later synced metadata could expose unreadable pointers.
• Relevant merged PR: #758.

2. The remaining problem is not basic correctness.

• The merged line completes end-to-end for both fast and wal_on_fast.
• The work is now about memory and maintenance effectiveness.

3. The runtime disk gap is still mostly maintenance debt.

• Post-maintenance floors remain much lower than runtime end-state.
• Small scheduler tweaks do not solve this robustly.

4. The remaining problem is no longer a missing constant.

• The small knobs have been exercised enough.
• The next useful step is a deeper maintenance algorithm change.

5. Assertion-driven refinement of the subsystem generating pathological directory structure is the right tactic.

• Do not only assert queue state.
• Assert filesystem shape and lane behavior.

Current Large-Change Direction

The next phase should be a deeper maintenance algorithm change:

Maintenance V2 target architecture

Build a unified incremental maintenance engine with:

1. debt ledger
2. incremental scanner
3. bounded executor
4. controller

Phase 1 target

The immediate next implementation target is:

• persistent debt ledger
• debt-aware rewrite selection
• bounded rewrite executor
• clean resume across reopen

Why this is the right scope

• resumable is not enough as the final destination
• incremental is the target architecture
• but implementation should still proceed in coherent phases, not a giant rewrite

Concrete Workstreams For Additional Agents

These are independent enough to parallelize.

Workstream A: l0 pathology / lane routing assertions

Goal:

• identify and constrain the subsystem generating unreasonable l0 growth

Likely suspects already identified:

• outer-leaf value-log appender hard-pinned to one lane
• small-count but huge-value pointer paths collapsing to one lane because fan-out is record-count based
• startup/open rotation and all-lane rotation paths

Good test targets:

1. Open does not rotate untouched split-vlog lanes
2. memtable/WAL rotation does not churn untouched lanes
3. a small number of very large pointer-backed values should not all collapse to one lane just because record count is below threshold
4. repeated maintenance opportunities should not create bounded synthetic tiny-file explosions on non-active lanes
5. l0 segment growth stays bounded under representative synthetic workloads

Workstream B: debt ledger / rewrite selection phase 1

Goal:

• replace simple queue-of-file-IDs behavior with a real persistent debt-aware ledger

Needed pieces:

1. persistent per-segment state
2. score / priority for rewrite candidate selection
3. cooldown / retry semantics
4. bounded rewrite executor using bytes/time/segment budget
5. reopen/resume persistence tests

Workstream C: telemetry-driven analysis

Goal:

• use current maintenance telemetry to determine where runtime debt is coming from on real run_celestia

Use the instrumentation landed around checkpoint kicks / queue stats / dry-run stats to answer:

• are rewrite kicks firing enough?
• is GC dry-run showing useful eligibility but not enough follow-through?
• is rewrite debt mostly accumulating in l0 because selection is wrong or because lane routing is wrong?

Workstream D: future incremental GC phase (not immediate)

Goal:

• after rewrite phase-1 is working, add resumable/incremental GC progress

Not the immediate next code change, but should remain the next architectural phase after rewrite selection/execution improves.

Recommended Validation Rules

Every candidate change in this area should be validated with:

1. exact-head run_celestia on fast
2. wal_on_fast sanity run when behavior may affect shared maintenance paths
3. compare:
   • duration_seconds
   • max_rss_kb
   • end_app_bytes
   • du -sb application.db
4. run:
   • treemap vlog-gc -rw
   • treemap vlog-rewrite -rw
5. compare maintained floor against runtime end-state

Do not keep a branch unless it improves the right combination of:

• correctness preserved
• runtime debt improved
• wall time not materially worse
• RSS not materially worse

Current Success Criteria

The next accepted phase should aim for:

1. fast still completes correctly
2. wal_on_fast still completes correctly
3. runtime du and end_app_bytes move materially closer to the post-maintenance floor
4. peak RSS does not regress, ideally improves
5. filesystem shape becomes saner:
   • fewer pathological l0 segments
   • bounded tiny-fragment churn on non-active lanes

Suggested Starting Point For A Future Agent

1. Start from merged #767, not an old scratch branch.
2. Treat #784 as proof that assertion-driven directory-shape fixes can expose real bugs.
3. Do not spend another cycle on micro-tuning scheduler constants.
4. Start with Workstream A plus Workstream B:
   • assertion-driven l0/lane-routing tests
   • debt-ledger rewrite phase-1 implementation
5. Use real Celestia runs as the acceptance gate.

[snissn]

Adding the remaining open Copilot item from #753 before final collapse to main:

• #753 TreeDB/caching/db.go:5660
  • waitForForegroundMaintenanceQuiet() is dead / unused and should either be removed or wired to a real call site.
  • Current disposition: defer to follow-on cleanup; not blocking the final correctness merge because the active fast/wal_on_fast head has already passed end-to-end run_celestia.

[snissn]

After review, the #751 retained-prune / generation-maintenance coupling item does not currently need a behavior change.

Key finding:

• scheduleRetainedValueLogPrune() and maybeRunVlogGenerationMaintenance() use the same foreground quiet-window gate.
• That means a scheduled-but-not-yet-running prune cannot block generation maintenance for the full retainedPruneQuietWindow.
• The only pending-only delay is the foreground maintenance poll interval (foregroundMaintenancePollInterval()), which is currently vlogGenerationLoopInterval / 10 = about 100ms.

Conclusion:

• keep the current serialization with waitForRetainedValueLogPrune() in maybeRunVlogGenerationMaintenance()
• document the reason in code
• do not carry this as an open follow-up blocker anymore

[snissn]

Started item 2 (#756 cleanup/design follow-up) in #765.

Current scope in the PR:

• preserve snapshot close errors in rewrite planning / rewrite paths instead of suppressing them
• collapse the rewrite-plan estimate hook registry to a single serialized test hook
• return the immutable cached live-bytes map directly on cache hits instead of cloning it

Local validation run:

• go vet ./TreeDB/db
• go test ./TreeDB/db -run "TestValueLogRewritePlan_(CachesLiveBytesForUnchangedState|InvalidatesCachedLiveBytesAfterCommit|SparseSelection_SelectsHighStaleSegment|GroupedPointers_DedupLiveBytes|NoSelectionKnobs_ReturnsTotalsOnly)"
• go test -race ./TreeDB/db -run "TestValueLogRewritePlan_(CachesLiveBytesForUnchangedState|InvalidatesCachedLiveBytesAfterCommit)" -count=10
• go test ./TreeDB/db -run "Test(ValueLogRewriteOffline_RewritesAndShrinks|ValueLogRewrite_LocalityPolicy_PreservesGroupedAdjacency|ValueLogRewriteOnline_UnsyncedRewriteThenVacuumRemainsReopenable)"

[snissn]

CI runtime follow-up landed in PR #766.

Results on a clean worktree from main under CI-like settings:

Command:

GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -json -p 1 ./TreeDB/caching -count=1 -timeout 20m

Before:

• package total: 455.543s
• top slow tests:
  • TestCachedRepeatedRewriteVacuumLeafRefsRemainReopenable_WALOn 152.440s
  • TestCachedRepeatedRewriteVacuumLeafRefsRemainReopenable 92.140s
  • TestCachedManualMaintenanceDirectPointersRemainReopenable_WALOn 47.130s
  • TestCachedGenerationalMaintenance_DirectPointersSeedPhaseLarge_WALOn 18.140s
  • TestCachedGenerationalMaintenance_DirectPointersManualGC_WALOn 18.140s
  • TestCachedGenerationalMaintenance_LeafRefsBackgroundReopenable_WALOn 17.460s
  • TestCachedGenerationalMaintenance_DirectPointersRemainInCurrentSet_WALOn 15.890s
  • TestCachedGenerationalMaintenance_DirectPointersBackgroundScheduler_WALOn 15.820s
  • TestCachingDB_PrunesRetainedValueLog 15.260s
  • TestVlogGenerationGC_ProtectsRetainedSegmentsBeforeFlush 15.140s

After:

• package total: 41.863s
• top slow tests:
  • TestCachedGenerationalMaintenance_DirectPointersBackgroundScheduler_WALOn 3.610s
  • TestCachedSeed_BatchSizeFillShouldBeHigh 2.760s
  • TestCachedBenchBloatVacuum 2.560s
  • TestCachedBenchBloat_PageCountRatioVsVacuum 2.450s
  • TestRaceFlushRotate 2.130s
  • TestCachedRewriteLeafRefs_RemainReopenableAfterLaterCheckpoint 2.110s
  • TestNoteWriteKeyDeadlock 2.000s
  • TestCachedGenerationalMaintenance_LeafRefsRemainReopenable 1.530s
  • TestRelaxedSync_Performance 1.100s
  • TestCachedRepeatedRewriteVacuumLeafRefsRemainReopenable_WALOn 0.890s

Main changes:

• backdate retained-prune quiet-window state in tests that are specifically validating prune behavior
• disable retained-prune in heavy maintenance/reopen integration tests where it is unrelated to the invariant under test
• reduce oversized WAL-on seed/round counts while keeping multi-generation coverage
• replace repeated local helper duplication with shared logical fixture helpers

This was a test-runtime reduction only. No product behavior was changed.

Remaining runtime hotspots are now the small group of ~2-4s tests listed above, which are much more manageable and can be optimized later if needed.

[snissn]

Started the next runtime-perf chunk on branch pr/762-resumable-maintenance-queue and opened PR #767: #767

What landed in this chunk:

• add persisted rewrite queue state in TreeDB/caching/vlog_generation_state.go
• cached scheduler now:
  • loads persisted rewrite SourceFileIDs
  • resumes queued rewrite work before replanning
  • processes one source segment per maintenance run
  • persists remaining queue across reopen
  • preserves the queue on rewrite failure
• added focused scheduler tests for:
  • resume without replanning
  • survive reopen
  • preserve queue on rewrite error

Local validation:

• GOWORK=off go vet ./TreeDB/caching
• GOWORK=off go test ./TreeDB/caching -run 'Test(VlogGenerationRewriteQueue_ResumesWithoutReplanning|VlogGenerationRewriteQueue_SurvivesReopen|VlogGenerationRewriteQueue_PreservedOnRewriteError|VlogGenerationRewrite_UsesAndConsumesBudgetedBytes|VlogGenerationRewrite_ConsumesBudgetToZeroWhenRewriteExceedsBudgetCap|VlogGenerationRewritePlan_DoesNotRunWithZeroBudgetTokens|VlogGenerationRewritePlan_RunsOutsideMaintenanceBarrier)' -count=1
• GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -p 1 ./TreeDB/caching -count=1 -timeout 20m

Important limitation:

• this is foundation only
• it does not change current Celestia runtime behavior yet, because the fast / wal_on_fast profiles still force ValueLog.Generational.Policy=off
• next step is to decide whether to enable the scheduler for those profiles, or otherwise route runtime maintenance through this resumable queue, then measure Celestia disk debt / RSS again

[snissn]

Checkpoint-kick follow-up on #767 is now pushed at 9a26aabb.

What changed:

• checkpoint-triggered resumable maintenance now calls the existing periodic GC path as well as rewrite
• still gated to DisableWAL, so this does not affect wal_on_fast
• added direct scheduler coverage for checkpoint-triggered GC despite recent foreground activity

Validation:

• GOWORK=off go vet ./TreeDB/caching
• focused scheduler/queue slice passed
• full TreeDB/caching package passed under CI-like settings
• end-to-end run_celestia fast completed successfully on stock iavl

Current measured fast progression on #767 line:

1. original #767 baseline

• run: /home/mikers/.celestia-app-mainnet-treedb-20260309184039
• duration_seconds=445
• max_rss_kb=24003872
• end_app_bytes=15491792414

2. rewrite-only checkpoint kick

• run: /home/mikers/.celestia-app-mainnet-treedb-20260309191742
• duration_seconds=294
• max_rss_kb=19566068
• end_app_bytes=15417566130
• manual maintenance on that run:
  • before: 15417566130 bytes
  • after vlog-gc -rw: 9952766851 bytes
  • after vlog-rewrite -rw: 2572970210 bytes

3. rewrite + periodic-GC checkpoint kick

• run: /home/mikers/.celestia-app-mainnet-treedb-20260309193315
• duration_seconds=288
• max_rss_kb=21372676
• end_app_bytes=14437980071
• audit: /tmp/767_gc_kick_audit.json
  • segments_on_disk=1290
  • bytes_on_disk=14224137062

Interpretation:

• this is a real partial win against the original #767 baseline:
  • wall time -157s
  • peak RSS -2.63 GiB
  • runtime app DB -1.05 GB
• compared with the rewrite-only kick, GC kick improves runtime disk debt but gives back some RSS
• runtime maintenance debt is still not solved: end-of-run disk is still far above the post-maintenance floor

So #767 now has a defensible partial improvement, but #762 is still open because the remaining runtime vlog debt is larger than this bounded kick can solve.

[snissn]

Follow-up on resumable maintenance PR #767:

I fixed a correctness bug exposed by the new checkpoint-triggered GC path.

Root cause:

• backend TreeDB/db.ValueLogGC() re-read db.valueLogManager multiple times
• backend DB.Close() nils valueLogManager before closing it
• checkpoint-kicked GC could race close and panic on nil receiver access inside CurrentSet() / CurrentSetNoRefresh()

Fix:

• snapshot the manager pointer once at the start of ValueLogGC() and use that stable pointer for the whole call

This does not change the runtime maintenance strategy, but it removes a new close-vs-GC race introduced by the checkpoint-kick path.

Validation:

• GOWORK=off go test ./TreeDB ./TreeDB/caching -run Test(ValueLogGCOnline_ProtectedSetSafety|Checkpoint_KicksVlogGenerationGCDespiteRecentForegroundActivity) -count=20\n- GOWORK=off go test ./TreeDB/db -run Test(ValueLogGC_RemovesUnreferencedSegment|ValueLogGC_ProtectedPathsKeepsSegment|ValueLogGC_OnlineMode_HealthStateNoUnsafeDeletes) -count=1\n- GOWORK=off go vet ./TreeDB/db ./TreeDB/caching ./TreeDB

[snissn]

Tried a bounded follow-up experiment on #767: raise vlogGenerationRewriteResumeMaxSegments from 1 to 2 so each resumable rewrite cycle processes two source segments instead of one.

Result: rejected.

Validation:

• focused queue/resume scheduler tests passed
• full TreeDB/caching package still passed locally
• real run_celestia fast run completed successfully

Run:

• /home/mikers/.celestia-app-mainnet-treedb-20260309195431
• duration_seconds=319
• max_rss_kb=25003696
• end_app_bytes=15322076126
• du_bytes=15322076126

Compared with the current #767 line before this experiment:

• prior best on branch: /home/mikers/.celestia-app-mainnet-treedb-20260309193315
• duration_seconds=288
• max_rss_kb=21372676
• end_app_bytes=14437980071

So the chunk-size increase made the branch worse on all three runtime metrics that matter:

• wall time +31s
• peak RSS +3.45 GiB
• runtime app DB +0.88 GB

Manual maintenance on the experimental run still collapsed the DB to the same approximate floor:

• before: 15.32 GB
• after vlog-gc -rw: 7.74 GB
• after vlog-rewrite -rw: 2.53 GB

Interpretation:

• larger resumable rewrite chunks do not solve the remaining runtime debt in a useful way
• they increase foreground cost without materially changing the maintained floor
• this lever should be considered explored and rejected for the current phase

[snissn]

Update on #767:

• Fixed the checkpoint-kicked maintenance crash path by requiring a fresh Checkpoint() before the kicked rewrite/GC scans run.
• Added regression assertions that checkpoint-kicked rewrite and GC each observe a second checkpoint boundary.
• Isolated direct maintenance tests that were unintentionally racing the checkpoint kick (TestIteratorTracksActiveForegroundIterators, protected-path rewrite tests).

Local validation passed:

• GOWORK=off go test ./TreeDB/caching -run 'TestCheckpoint_KicksVlogGeneration(Rewrite|GC)DespiteRecentForegroundActivity' -count=20\n- GOWORK=off go test -race ./TreeDB -run 'TestValueLogGCOnline_ProtectedSetSafety' -count=20\n- GOWORK=off go test -race ./TreeDB/caching -run 'TestCheckpoint_KicksVlogGeneration(Rewrite|GC)DespiteRecentForegroundActivity' -count=20\n- GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -p 1 ./TreeDB/caching -count=1 -timeout 20m\n\nThis is a correctness/stability fix for the current resumable-maintenance PR, not a new runtime-perf win by itself.

[snissn]

Additional #767 stabilization update:

• isolated the auto-checkpoint tests from checkpoint-kicked maintenance
• these tests are only about WAL auto-checkpointing, so the new checkpoint-kick path was introducing unrelated package-order noise under -race

Validation:

• GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -race -p 1 ./TreeDB/caching -count=1 -timeout 20m passed locally after the change
• GOWORK=off go vet ./TreeDB ./TreeDB/caching passed locally

This was in response to the remaining race-check (linux) failure on #767.

[snissn]

Current #767 head dc956810 is review-clean (unresolved=0) and has passed another real run_celestia validation on fast:

• run: /home/mikers/.celestia-app-mainnet-treedb-20260309204844
• result: success
• this run crossed restore, entered live blocks, and completed without the earlier first-live-block failures

The latest code changes on this head were:

• drain checkpoint-kick maintenance through db.wg so Close() cannot race backend teardown
• factor duplicated atomic write / Windows rename retry logic into TreeDB/internal/atomicfile
• isolate auto-checkpoint tests from the checkpoint-kick path

CI is still draining on this head (bad=0, some checks still pending), so I have not made a merge/keep decision yet.

[snissn]

Another real run_celestia validation on current #767 head dc956810 succeeded on fast:

• run: /home/mikers/.celestia-app-mainnet-treedb-20260309204844
• duration_seconds=396
• max_rss_kb=21064308
• end_app_bytes=20635671029
• du -sb application.db = 11577528934

This confirms correctness on the latest head after the checkpoint-kick drain / atomic-write-helper changes, but it also confirms that runtime maintenance debt is still large on this line. The branch remains a correctness-positive partial win, not the final perf answer.