fix(storage): cap LMDB map head-room on Windows to bound page-table memory

[jacderida]

Linear issue

V2-620

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Node storage logic with a behavioural surface — routine resize-on-demand and the
associated env_lock discipline — but no change to the wire protocol, stored-data
format, payments/economics, or the upgrade mechanism. Classified T2 per review; ADR
linked below.

Compatibility

• Wire: none
• Storage: none — no on-disk format change; existing databases open unchanged (verified by reopening a live 22.6 GiB store under the smaller map).
• API: none

Semver impact

• breaking
• feature
• fix

Test evidence

• Unit tests map_target_covers_existing_data_and_headroom and
  map_target_never_truncates_data_when_disk_nearly_full assert the Windows
  head-room cap binds, that existing data is always covered even when the head-room
  term saturates to zero, and that non-Windows platforms keep the disk-sized map.
  cargo test and cargo clippy (repo -D panic/unwrap/expect) are clean.
• Resize-safety regression test read_paths_block_while_env_is_being_resized
  asserts all_keys/get_raw hold the shared env_lock and block until an
  in-progress resize (exclusive lock) is released.
• Live before/after on a Windows production node holding 22.6 GiB of chunks:
  • LMDB map size: 3,719 GiB → 54.58 GiB
  • Commit / Private: 7,537 MB → 181 MB
  • Virtual size: 3,719 GB → 60 GB
  • The node reopened its existing 22.6 GiB database under the smaller map (no data
    loss), and page-table memory fell ~40×. Projected to the reporting user's 3-node
    host, ~60 GB collapses to well under 1 GB.

New dependency

none

ADR

ADR-0007: Cap the Windows LMDB map head-room

Mitigation / rollback

Revert the single commit — the change is Windows-only (#[cfg(windows)]) and
behaviourally transparent (existing DBs open unchanged; the resize-on-demand path is
pre-existing and already exercised), so there is no data migration to undo.

[Copilot]

Pull request overview

This PR addresses excessive, unaccounted-for kernel memory consumption on Windows caused by LMDB mapping “disk-sized” regions (triggering eager page-table allocation), by capping the auto-sized map head-room on Windows and relying on on-demand resizing as data grows.

Changes:

• Add a Windows-only cap (WINDOWS_MAP_HEADROOM = 32 GiB) to bound LMDB map head-room and page-table overhead.
• Refactor map sizing logic into a unit-testable helper (map_target_bytes) and update compute_map_size to use it.
• Add unit tests covering the Windows cap behavior, “never truncate existing data” behavior, and non-Windows behavior.

Comments suppressed due to low confidence (1)

src/storage/lmdb.rs:749

• This doc comment links to WINDOWS_MAP_HEADROOM, but that constant is only defined on Windows (#[cfg(windows)]), so the link will be broken in non-Windows rustdoc builds. Prefer backticks (or define a non-Windows stub constant) to avoid broken intra-doc links.

/// `map = current_db_bytes + max(0, available − reserve)`, with the head-room
/// term capped at [`WINDOWS_MAP_HEADROOM`] on Windows. Existing data
/// (`current_db_bytes`) is always covered so a resize can never truncate the

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dirvine]

Review summary

Reviewed exact head 00243192ab001c6a44237a4cc83e6812b822970c with the review panel and local verification.

Verdict: changes requested — one merge-blocking LMDB resize-safety issue.

Blocking: env_lock does not cover every active LMDB transaction

try_resize() takes env_lock.write() and then calls unsafe Env::resize(), with the stated invariant that no transactions are active (src/storage/lmdb.rs:652-678). However, two production read paths open transactions without taking the matching shared lock:

• all_keys() — src/storage/lmdb.rs:546-573
• get_raw() — src/storage/lmdb.rs:588-603

Heed/LMDB explicitly requires that mdb_env_set_mapsize be called only when no transactions are active; the library does not enforce this for readers. The resize implementation can unmap/remap the environment while either untracked transaction or cursor still references it.

The omission predates this PR, but this change makes resize-on-demand a normal Windows lifecycle path (approximately each additional 32 GiB) rather than an exceptional partition-expansion path, so the race becomes materially reachable during concurrent replication/audit activity.

Required fix: clone env_lock into both blocking closures and hold lock.read() from before read_txn() until the transaction, cursor and copied LMDB values have all been dropped. Please also add a regression test proving resize waits for a held all_keys/get_raw transaction before entering env.resize().

Non-blocking review notes

• The reported Windows before/after figures are internally coherent and strongly support the operational fix. The precise explanation in the comments is more certain than the evidence: the pinned LMDB backend uses a reserved file view, and modern Windows does not generally materialise one hardware PTE per untouched 4 KiB page. I suggest describing the observed effect as size-proportional Windows VM/section metadata or commit/private overhead unless the exact kernel allocation was measured.
• T1 is labelled “client-only” in the PR template, while this changes node storage behaviour and deliberately relies on routine resize-on-demand. Please reconsider T2/ADR classification or record an explicit policy exception.
• At review time, the Ubuntu, macOS and Windows test jobs were still pending. The Windows job is material because it executes the #[cfg(windows)] arithmetic branch.

Verified

• cargo fmt --all -- --check — pass
• cargo test storage::lmdb --lib --no-fail-fast — 13 passed, 0 failed
• Windows, macOS and Ubuntu builds, Clippy, docs, formatting, security audit and no-logging tests — pass
• Current PR head revalidated immediately before this review

No merge performed.

[jacderida]

Thanks for the thorough review, @dirvine — the resize-safety catch is exactly right. Addressed in e44171f.

Blocking — env_lock coverage on read paths (fixed)

all_keys() and get_raw() now take env_lock.read() for the full duration of their read transaction — the guard is acquired before read_txn() and held until the transaction, cursor, and copied bytes are all dropped — matching try_put/get/delete and the two sync readers. Since try_resize() acquires env_lock.write() before the unsafe Env::resize(), a resize now waits for any in-flight all_keys/get_raw transaction.

Regression test read_paths_block_while_env_is_being_resized: it holds the exclusive lock (as a resize does) and asserts both all_keys() and get_raw() block until it is released, then complete and see the data. It fails against the pre-fix code (the read paths took no lock and returned immediately).

Non-blocking — mechanism wording (done)

Agreed the per-PTE explanation was more precise than the evidence supports. The doc comment and commit message now describe the observed effect as size-proportional Windows commit/private overhead (~0.2% of mapped size — measured: ~7.4 GB at a ~3.7 TiB map vs ~181 MB at ~55 GiB) and explicitly note the exact kernel structure was inferred from that scaling, not measured directly. (We intended to confirm via RAMMap's Page Table row, but the node was stopped before that capture — happy to gather it on a follow-up if useful.)

Non-blocking — risk tier / ADR

Fair point — I flagged the T1-vs-T2 call in the PR body but you're right that routine resize-on-demand makes it node-behavioural. Confirming the classification with the author now; I'll update the tier box and add an ADR if it lands at T2 before marking this ready.

CI

The build/test/clippy jobs (including the Windows job that exercises the #[cfg(windows)] arithmetic branch) are re-running on e44171f; I'll confirm they're green.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[jacderida]

Re: risk tier — agreed, reclassified to T2. Added ADR-0007: Cap the Windows LMDB map head-room (docs/adr/ADR-0007-windows-lmdb-map-headroom-cap.md, in 43665bf) and linked it in the PR body; the tier box is now T2.

The ADR records the context, the options weighed, the resize-cadence trade-off, and the env_lock discipline the design depends on (including the all_keys/get_raw fix). Status is Proposed pending your review.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dirvine]

Follow-up review

Reviewed exact head 43665bf35562000128d30e057d4448d342eceb94 after the requested changes.

The previous resize-safety blocker is resolved:

• all_keys() and get_raw() now hold env_lock.read() across their complete LMDB transaction/cursor or borrowed-value lifetime.
• All runtime LMDB transaction paths are covered by the same shared lock; try_resize() takes the exclusive lock before Env::resize().
• The regression test passed locally 20/20 repetitions; cargo test storage::lmdb --lib --no-fail-fast passed 14/14 and formatting passed.
• The mechanism wording was corrected, the PR is classified T2, and ADR-0007 records the policy and lock invariant.

Review-panel consensus found no material blocker. One reviewer preferred a stronger reader-first test hook to prove the full guard lifetime; source inspection establishes that lifetime directly, so I regard this as non-blocking.

All current-head checks are green on rerun, including macOS, Ubuntu and Windows tests. The first Windows E2E attempt had an unrelated transport-initialisation failure; the rerun passed.

Approved. No merge performed.