Skip to content

Avoid duplicate internal-page materialization in leaf-pack publication #3715

Description

@snissn

Follow-up to #3666 and PR #3709; descendant of the completed maintenance graph #3630.

Goal

Remove duplicate internal-page materialization from leaf-generation pack publication.

Done means the pack path prepares an immutable relocation plan before acquiring writeMu, exact basis revalidation still occurs before publication, the exclusive path binds committed page IDs and writes each internal page once without recursively decoding and rebuilding staged pages, and the paired performance and correctness gates below pass.

This is a single-node continuation of the #3666 workstream. It does not reopen #3630 or #3666.

Current Evidence

PR #3709 reduced the fixed leaf-pack apply cost on its accepted paired benchmark:

Metric Before After Delta
pack apply 81.18 ms/op 47.59 ms/op -41.38%, p=0.002
private copy 39.07 ms/op 19.50 ms/op -50.1%
publish hold 40.87 ms/op 27.02 ms/op -33.9%
PL-01 p95 / p99 baseline contract 1.05x / 0.97x within the <=2x gate

The current implementation on origin/main at d1b2d909e still performs two representations of rewritten internal pages:

  1. Private rewrite builds staged internal pages in a pager.NewOverlay.
  2. After taking writeMu and revalidating the exact basis, cloneLeafGenerationPackStagedNode recursively calls staged Get, verifies checksums, parses internal entries, allocates committed pages, rewrites child references, and rebuilds nodes with node.Builder.

This second recursive decode/build is inside the exclusive publication window and is specific to leaf-pack relocation. Existing structured stage counters expose TreeRewriteTimeNanos, RelocationTimeNanos, page sync, registration, finalization, and related stages, but BenchmarkCompactStorageLeafPackMultiPass currently reports only coarse apply/copy/publish metrics.

Root-Cause Classification Gate

Before selecting or retaining the implementation, report all relevant ApplyStages fields in the existing multi-pass benchmark and collect a current pinned-CPU baseline.

Classification Evidence Required action
duplicate staged parse/build TreeRewrite + Relocation dominate local CPU/wall and recursive staged-node cloning is exercised implement prepared relocation templates as specified below
private rewrite dominates TreeRewrite dominates while relocation is minor optimize only a directly profiled duplicate materialization cost; retain the same gates
relocation is noise relocation is too small to support the required gates stop implementation, publish evidence, and keep this issue open for a newly profiled local fix
durability syscall/wait dominates page/directory/meta sync dominates treat it as an excluded adjacent lane; do not weaken or batch barriers in this issue

Instrumentation-only benchmark changes are allowed before the red test. The optimization itself must start from a deterministic failing test.

Required Fix

  • During private rewrite, prepare an immutable relocation plan for every reachable staged internal page. The plan must preserve page type, entry ordering, keys, values, flags, and child-reference positions without assigning committed page IDs.
  • Deduplicate shared staged pages by staged page ID. User root, system/collection root, and shared descendants must resolve through one plan entry and one final committed page.
  • After acquiring writeMu, preserve the existing exact basis revalidation and retry/abort behavior. A failed revalidation must discard the entire plan; no committed page IDs or retired IDs from an aborted attempt may be reused.
  • Only after successful revalidation, allocate committed IDs, resolve child references, set final page IDs/checksums, and write each prepared page once. The locked publication path must not recursively decode and rebuild staged internal nodes.
  • Preserve existing page sync, directory sync, registration, collection-root publication, metadata finalization, source-generation pinning, poisoning, and cleanup ordering.
  • Preserve the public stats/JSON surface. Add benchmark reporting for detailed stage fields without renaming or removing existing metrics.

An equivalent implementation is acceptable only if it proves the same no-recursive-rematerialization property and passes every gate below.

Test-Driven Development

  • Add a deterministic red test or test-only counter/hook proving the current locked publication path recursively materializes staged internal pages, then assert the replacement path performs zero recursive staged internal-page rebuilds.
  • Verify prepared and published pages are semantically equivalent for user roots, system/collection roots, multi-level trees, and shared child references.
  • Verify each staged internal page maps to exactly one committed page and all rewritten child references target the mapped committed IDs.
  • Verify checksum corruption in staged input still fails closed before publication.
  • Verify a concurrent foreground commit causes a full abort/retry and the discarded plan contributes no stale page IDs, retired IDs, or root publication.
  • Keep promotion, directory-sync, registration, metadata-write, metadata-sync, cleanup, and poisoning failure seams green.
  • Keep checkpoint/close/reopen, leaf-generation GC, and persistent value-log pointer tests green.
  • Keep detailed stage accounting non-negative, bounded by apply/publish wall, retry-separated, and JSON compatible.

Focused minimum validation:

GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -p 1 ./TreeDB/db \
  -run 'TestLeafGenerationPack|TestCompactStorage.*LeafPack' -count=1
GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -race -p 1 ./TreeDB/db \
  -run 'TestLeafGenerationPack_(Retry|Copy|Changed|Private|Source|SyncFalse)' -count=3
GOWORK=off GOMEMLIMIT=4GiB GOMAXPROCS=2 go test -p 1 ./TreeDB/pager -count=1

Run broader affected-package tests on the exact PR head. Any failure in durability, reopen, GC, retry, or race coverage blocks merge.

Performance Gates

Use the current origin/main baseline, not PR #3709's historical numbers, as the authoritative comparison. Keep benchmark source and fixture byte-identical, pin the same isolated CPU, set the same GOMAXPROCS and memory limit, and run at least six interleaved base/head pairs. Publish raw outputs, source hash, exact SHAs, command, CPU/hardware context, medians, and benchstat.

Gate Target Failure action
TreeRewrite + Relocation >=20% lower continue profiling/implementation; do not close
pack apply wall >=10% lower continue profiling/implementation; do not close
publish hold no regression beyond 5% block merge
allocations and bytes each no regression beyond 5% optimize or explain host noise with repeat evidence; otherwise block
total CompactStorage wall directional improvement and no material regression block or isolate demonstrated host noise
PL-01 foreground-write p95/p99 each <=2x idle baseline block merge
durability/correctness all focused, race, reopen, GC, and exact-head CI pass block merge

The historical #3709 apply result implies an indicative target below about 42.8 ms/op, but only the newly measured current baseline controls acceptance.

Non-Goals

Branch, Review, And Merge Gates

  • Use a topic branch from the recorded origin/main base in an isolated worktree.
  • Keep benchmark instrumentation separable from the optimization so the paired binaries can use byte-identical benchmark source.
  • PR body must include the root-cause table, red/green test evidence, exact benchmark commands and artifacts, base/head SHAs, benchmark source hash, before/after table, and regression assessment.
  • Request Codex and repository review only after the code, tests, and paired evidence are mature.
  • Require latest-head CI, an exact-head AI review artifact, zero unresolved material review threads, and a clean merge state before merge.
  • If a gate fails, keep the issue in fix-needed state and iterate. Do not close it as insufficient without an explicit maintainer waiver.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions