fix(storage): treat a node as full only when disk and store are both out of room - #210
Open
grumbach wants to merge 2 commits into
Open
fix(storage): treat a node as full only when disk and store are both out of room#210grumbach wants to merge 2 commits into
grumbach wants to merge 2 commits into
Conversation
…out of room A node decided it was full from `fs2::available_space()` alone and never asked LMDB whether the write would actually fit. Deleting a record returns its pages to LMDB's free list and never to the filesystem, so a node that has pruned heavily sits on reusable capacity while `statvfs` still reports the volume as full. It refused every write anyway, including writes that would land in a freed page without growing `data.mdb` by a byte. On the production fleet one host crossed the 500 MiB reserve and logged 423,202 write rejections in six hours across all 13 of its nodes, having pruned 4,609 records in the preceding day. The store had room; the guard could not see it. The predicate now has both halves. Below the reserve the map is pinned to the file's high-water mark, so a put succeeds exactly when LMDB can serve it from the free list and returns `MDB_MAP_FULL` the moment it would extend the file. The allocator is the authority, not an estimate: no page count can account for the copy-on-write of the B-tree path, the contiguous run a multi-megabyte value needs, or pages still pinned by an open read transaction. `check_capacity` remains a cheap pre-check and stays biased towards admitting. It estimates reusable bytes from `env.stat()` and refuses only when there is not one chunk's worth, preserving the saving of rejecting a full node before payment verification without blinding one that still has room. It deliberately avoids heed's `non_free_pages_size()`, which walks the unnamed database calling `String::from_utf8(key).unwrap()` and so panics on 32-byte binary keys. Two hazards the pinned mode introduces are handled explicitly: - A delete is itself a write. On a store with no free page it cannot copy-on-write inside a map pinned to the file size, so the node could never prune its way out. `delete` raises the ceiling by a budgeted allowance, retries, and restores it inside one exclusive-lock scope, with RAII guards so neither the ceiling nor the allowance can leak on an unwind. The allowance is charged only when the delete commits, because that is the only outcome whose copy-on-write can have extended the file permanently. - A `spawn_blocking` body outlives a cancelled awaiter, so an async lock cannot order two resizes. The mode's intent is published before the work, and both resize closures re-read it under the exclusive lock and decline if it has since been reversed. `try_resize` also measures the disk inside the closure, so a late one sizes from the disk as it is rather than as it was. Reviewed adversarially over six rounds; the findings on cross-size verdict caching, permanent map slack, transition races, torn reads and leak paths are all addressed.
grumbach
force-pushed
the
fix/capacity-check-lmdb-reuse
branch
from
August 20, 2026 09:26
b73a09a to
154863d
Compare
A store pinned to its file size cannot always copy-on-write a delete, so the delete path offers a temporary ceiling raise. That raise was budgeted one grant per low-disk episode, on the assumption that the first assisted delete frees pages the next one reuses. That assumption is wrong. LMDB will not hand back pages a still-recent transaction freed, so consecutive deletes on a full store can each need a little room. Charging per grant therefore stopped a node pruning after its first assisted delete, which is the opposite of what the allowance exists for. It passed locally and failed in CI because the two differ in page size: 16 KiB pages left enough slack in the first grant to cover later deletes, 4 KiB pages did not. What needs bounding is permanent file growth, since LMDB never returns file space, not the number of times slack was offered. The allowance is now charged the bytes `data.mdb` actually gained, measured across the delete. A delete that finds room inside the file costs nothing and pruning continues indefinitely, while repeated fill-then-delete cycles are still stopped from walking the file into the disk reserve. The test that pruned a pinned store now also asserts the accounting rule directly, so a regression to per-grant charging fails on any page size rather than only on hosts with small pages.
grumbach
added a commit
to grumbach/ant-node
that referenced
this pull request
Aug 21, 2026
…e predicate The replication verification cycle gates its close-group probe on `LmdbStorage::capacity_verdict`, while `execute_single_fetch` gates the dial on `LmdbStorage::check_capacity`. The two restate one comparison rather than sharing it, so they can drift apart, and a verdict stricter than the pre-check is the harmful direction: a node the pre-check would let write stops discovering holders for keys it could have stored, which is under-replication rather than a saved probe. That is not hypothetical. WithAutonomi#210 makes the pre-check two-part, below the reserve and out of reusable pages inside the store, because LMDB returns a deleted record's pages to its own free list and never to the filesystem. A heavily pruned node fails only the first half, so with WithAutonomi#210 in and the verdict left as it stands, such a node stands its keys down for five minutes at a time instead of looking for chunks it can store. Add a unit test built on that state rather than on a bare full disk, where the two predicates still agree and a test would pass straight through the divergence. It writes and deletes two chunks so the store carries more than one chunk of reusable space, establishes the below-reserve precondition without either function under test, and asserts the two refuse together. Applying WithAutonomi#210's predicate to the pre-check fails this test and nothing else in the 934-test suite. Record the coupling in ADR-0011, as a trade-off, a validation entry and a review trigger, so the constraint outlives the pull request that found it.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear issue
https://linear.app/autonominetwork/issue/V2-1034/capacity-check-ignores-reusable-lmdb-pages-so-pruned-nodes-refuse
Risk tier
This is a bug fix restoring the intended capacity check, not a new design: the two-part predicate ("full" = no disk and no reusable page) was always what the guard was supposed to express, and the
MapFullhalf was already implemented and simply unreachable. No wire message, stored format, payment path or upgrade mechanism changes, and behaviour above the disk reserve is byte-for-byte unchanged.Reviewer note: the only behavioural delta is on a node already under its reserve, which today refuses every write. Flag it if you read that as T2.
Compatibility
check_capacitykeeps its signature and itsInsufficient disk space …error text; only the condition under which it fires changes.Semver impact
Test evidence
cargo test --lib: 936 passed, 0 failed.cfd(fmt + clippy + doc) clean.Seven tests added or rewritten, each pinned to a claim rather than to the implementation:
below_reserve_put_reuses_freed_pagesdata.mdbdoes not grow doing it.below_reserve_refused_put_does_not_grow_the_filelarge_refusal_does_not_block_a_smaller_putfull_store_below_reserve_can_still_deletecheck_capacity_tracks_reusable_space_not_just_diskstatvfsunchanged throughout.leaving_no_growth_restores_head_roomabove_reserve_behaviour_is_unchangedtest_put_rejected_on_insufficient_capacity_before_verification(handler) still proves a genuinely full node short-circuits ahead of payment verification, which is the saving the earlier pre-check work introduced.Adversarially reviewed over six rounds. Findings addressed rather than argued: a store-wide
MapFullverdict that let one large chunk lock out small writes; a permanent map slack that was really ordinary put capacity and multiplied by nodes per volume; transition races between the mode flag and the map size; a torn read in the reusable estimate that could under-report; and leak paths for both the raised ceiling and the delete allowance under error, panic and cancellation.Not yet done: no testnet run. Worth exercising on a deliberately filled host before it rides a train, because the behaviour only differs once a volume is under its reserve.
New dependency
none
ADR
n/a — bug fix, not an architectural decision.
The reasoning that would have gone in one is in the commit message and in the code comments at the decision points: why the allocator answers "does this write fit" instead of a page-count estimate, why
non_free_pages_size()must not be called, why a delete needs a budgeted allowance, and why an async lock cannot order two resizes.Mitigation / rollback
Revert the commit. The change is confined to
src/storage/lmdb.rsand one handler test, adds no state that outlives the process, and writes nothing new to disk. Behaviour above the disk reserve is unchanged, so the blast radius is limited to nodes already under their reserve, which today refuse every write regardless.