fix(clean,gc): detect reflinked sharing and explain empty evictions (#602, #509) - #641
Merged
Conversation
jleni
force-pushed
the
fix/clean-reflink-and-gc-grace
branch
from
July 31, 2026 20:02
5dafeb3 to
d763b61
Compare
…602, #509) Two independent reporting bugs, both of which make kache look broken while it is working correctly. #602: `clean` classified an artifact as cache-backed with `nlink > 1`, which only detects the hardlink fallback. On APFS, btrfs and XFS — the path `link_to_target` prefers — a restore is a clonefile/FICLONE reflink, which by design produces an independent inode with nlink == 1. So on the filesystems where kache works best, every restored artifact looked local and `clean` reported ~0% cached. That is the number the README advertises and the one a user leans on when deciding what to delete. New `src/sharing.rs` asks the filesystem the question that was actually meant: does this file share storage, and how much would deleting it really free. macOS uses getattrlist with ATTR_CMNEXT_PRIVATESIZE and ATTR_CMNEXT_EXT_FLAGS; Linux uses FS_IOC_FIEMAP and sums the extents without FIEMAP_EXTENT_SHARED. `nlink` is retained and OR'd in, so the non-CoW fallback keeps working and the two signals combine rather than one replacing the other. Measured on the reporter's own machine: `kache clean` on a 20.4 GiB target tree went from "0 B cached" to "11.1 GiB cached", and a whole-store scan reports 68.2% of bytes shared with live targets (the issue measured 62.3% before the store grew). `compute_link_stats` now counts reflinked blobs in `saved_bytes` too, where a purely reflinked store previously reported zero. Every probe failure degrades to "not shared, all bytes private" — the same answer the nlink-only code gave. An un-interrogable filesystem is therefore no worse off than before, and the reclaim estimate stays conservative. #509: `kache gc` printed a bare "evicted 0 entries" next to a store at 912% of its limit. Correct behaviour, uselessly reported: every candidate was inside EVICTION_IDLE_GRACE, so the sweep deliberately left it alone. A user seeing 0 beside 912% concludes GC is broken and stops trying, which is plausibly how #497 became a 113 GB bug report instead of a self-service fix. GcStats now carries entries_pinned, counted where the sweep already distinguished a grace skip from a removal, and `describe_eviction` turns the outcome into a sentence that names the grace period and says what to do next. Kept pure so the phrasing is unit-testable without running a sweep. Two portability notes, both verified by cross-compiling rather than assumed. The macOS attribute values are transcribed from the SDK header: they pack in ascending bit order, so PRIVATESIZE (0x008) precedes EXT_FLAGS (0x200), and EF_SHARES_ALL_BLOCKS is 0x40 — 0x02 is EF_NO_XATTRS, which would have marked every file without xattrs as shared. The first draft got both wrong and the reflink test caught it. On Linux the ioctl request is c_ulong on gnu but c_int on musl, so the FIEMAP constant is held as u32 and cast at the call site.
…allers `Test (Windows)` and `E2E smoke (Windows)` failed to compile with four dead-code errors (`-D dead-code` implied by `-D warnings`): Sharing, unknown_for, probe and probe_impl are all unreachable there. Both callers — `clean`'s classifier and `compute_link_stats` — are cfg(unix), because the signal they combine the probe with is `nlink`. Windows ReFS block-cloning has no read-side query to implement this against (FSCTL_DUPLICATE_EXTENTS_TO_FILE is write-only), so the module compiles to its unknown-answer stub and nothing calls it. It went unnoticed locally because the `#[cfg(test)]` tests do call `probe`, so a test build on any platform sees it as live; only a non-test Windows build is affected. Verified by reproducing the exact condition — private module, no callers, x86_64-pc-windows-msvc, -D warnings — which yields precisely CI's four errors without this attribute and compiles clean with it.
The mutation lane timed out at 45 minutes with 39 survivors. They split three ways, and only one of the three was a real coverage gap. Genuinely untested, now tested: - describe_eviction never saw a sweep with nothing pinned, so the grace-period paragraph could have been printed unconditionally. - artifact_sharing never saw a hardlink, so the nlink fold could have been inverted without a test noticing. - The over-limit comparison behind the two "evicted 0" messages is now a named function, store_over_limit, tested at, above and below budget and for an unreadable size — the same reason describe_eviction is pure. Unreachable on the lane's filesystem, now testable anyway: the reflink branch of compute_link_stats only runs where the filesystem can make a reflink, which ext4 cannot. Its bookkeeping moves into record_reflink_sharing, which takes the probe result as a parameter and is tested directly, including the saturating case where a probe reports more private bytes than the file's size. Likewise in sharing.rs: fiemap_window_length is split out because the loop only runs a second time for a file with more than 32 extents, which no test can arrange reliably; and a new sparse-file test gives the Linux probe a case where the honest answer differs from the fallback, so a probe that quietly gave up is now visible. Neither needs reflink support. Not observable at all, excluded with a reason: the mutation lane runs on ubuntu-latest, so mutants inside probe_macos and probe_unsupported are never compiled there and survive by construction. The platform impls are renamed so the exclusion can name them precisely and leave probe_linux — the one the lane does compile — fully in scope. Killing the survivors should also settle the timeout: a caught mutant stops at the first failing test, while a survivor runs the suite to completion, and excluding the never-built macOS impl removes 24 mutants from the set outright.
The lane now finishes in 34 minutes rather than timing out, and is down to eight survivors. This clears them. FIEMAP flag handling moves into extent_is_shared, extent_is_last and mapped_nothing, tested against synthetic flag words. A genuinely shared extent needs a filesystem that can make a reflink, which the lane's ext4 cannot, so the bit arithmetic is the part worth testing and it now is — including that a neighbouring flag (ENCODED, 0x0800) is not mistaken for SHARED, which would report private storage as already cached. The pinned-entry counter in apply_eviction is asserted by the existing grace-window test. It was the one number nothing checked, and it is the whole difference between "evicted 0 entries" reading as a broken GC and explaining itself. The three remaining macOS survivors were struct-field mutants on the getattrlist AttrList literal. exclude_re cannot reach them: cargo-mutants 27.1.0 does not apply the filter to struct-field mutants at all, which I confirmed by listing with --exclude-re on patterns as broad as the file name. Spelling the literal out in full removes the ..Default::default() they need to exist, which is better for an FFI struct anyway — the zeroed slots are now visible rather than implied.
…uses
`Check (Linux)`, `Nix package` and `E2E smoke (Linux)` were all failing at
their build step on the same two errors:
error: constant `FIEMAP_EXTENT_LAST` is never used
error: constant `FIEMAP_EXTENT_SHARED` is never used
`-D dead-code` comes in via `-D warnings`, so on Linux these are hard
errors rather than warnings. They are invisible on macOS, where the whole
`#[cfg(target_os = "linux")]` block is never compiled.
Extracting `extent_is_shared` and `extent_is_last` — so the bit tests
could be unit-tested against synthetic flag words — moved both constants
into those helpers. `probe_linux` now calls the helpers instead of
testing the bits inline, which left its own two declarations orphaned.
Removed them; the remaining declarations live in the functions that
actually read them.
Also folds this branch's mutants.toml exclusions together with the
`run_config_editor` exclusion that landed on main, so all four reasons a
mutant is unobservable rather than untested sit in one list.
Verified on macOS: fmt, clippy -D warnings, 1458 tests. The dead-code
lint itself is Linux-only and could not be reproduced locally —
cross-compiling the C dependency tree (ring, zstd-sys, libsqlite3-sys,
blake3) is not worth it for a two-line deletion — so CI is the check for
that specific error. The deletion is auditable without it: nothing else
in the tree references either constant.
jleni
force-pushed
the
fix/clean-reflink-and-gc-grace
branch
from
August 2, 2026 17:44
c4e0ad6 to
5444faf
Compare
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.
Closes #602. Closes #509.
Two independent reporting bugs, both of which make kache look broken while it is working correctly. Grouped because they are both
area:localoutput-honesty fixes and both small.#602 —
cleanreported ~0% cached on the filesystems kache is best oncleandecided an artifact was cache-backed withnlink > 1, which only detects the hardlink fallback. On APFS, btrfs and XFS — the pathlink_to_targetprefers — a restore is aclonefile/FICLONEreflink, which by design produces an independent inode withnlink == 1. Every restored artifact therefore looked local. That is the number the README advertises and the one a user leans on when deciding what to delete.New
src/sharing.rsasks what was actually meant: does this file share storage, and how much would deleting it really free.getattrlist+FSOPT_ATTR_CMN_EXTENDED→ATTR_CMNEXT_PRIVATESIZE,ATTR_CMNEXT_EXT_FLAGSFS_IOC_FIEMAP, summing extents withoutFIEMAP_EXTENT_SHAREDnlinkis retained and OR'd in rather than replaced, so the non-CoW fallback keeps working and the two signals combine.Measured on the reporter's machine:
A whole-store scan reports 68.2% of bytes shared with live targets, against the 62.3% measured on the issue before the store grew.
compute_link_statsnow counts reflinked blobs towardsaved_bytes, where a purely reflinked store previously reported zero.Failure direction is deliberate. Every probe failure degrades to "not shared, all bytes private" — exactly what the
nlink-only code returned. A filesystem we cannot interrogate is no worse off than before, and the reclaim estimate stays conservative: claiming bytes are shared when they are not would understate what a delete frees, which is the direction that makes a user keep files they could have removed.#509 —
evicted 0 entriesnext to a store at 912%Correct behaviour, uselessly reported. Every candidate was inside
EVICTION_IDLE_GRACE, so the sweep deliberately left it alone. A user seeing0beside912%concludes GC is broken and stops trying — plausibly how #497 became a 113 GB bug report rather than a self-service fix.GcStatsgainsentries_pinned, counted at the point where the sweep already distinguished a grace skip from a removal.describe_evictionturns the outcome into a sentence that names the grace period and says what to do next. Kept pure, so the phrasing is unit-testable without running a sweep.Portability, verified rather than assumed
The macOS attribute values are transcribed from the SDK header. They pack in ascending bit order, so
PRIVATESIZE(0x008) precedesEXT_FLAGS(0x200) — andEF_SHARES_ALL_BLOCKSis0x40, where0x02isEF_NO_XATTRSand would have marked nearly every file as shared. The first draft got both wrong; the reflink test caught it, and an independent C probe using the same syscall confirmed the corrected reading (priv=0 ef=0x41on a restored.rlib). This is the same ascending-bit-order trap that produced the numbers later corrected on the issue.On Linux the ioctl request is
c_ulongon gnu butc_inton musl, so the FIEMAP constant is held asu32and cast at the call site. Caught by cross-compiling, not by inspection.Testing
fmtclean;clippy --workspace --all-targets -- -D warningsclean, verified with a forced recompile rather than a cached result.-D warningsforaarch64-unknown-linux-gnu,aarch64-unknown-linux-musl,x86_64-pc-windows-msvc.kache cleanagainst a real 20.4 GiB tree and a 48 GiB store.Known gap: the Linux FIEMAP path compiles on both libcs and is written to fail safe, but I could not execute it — no working container runtime on this machine. It remains untested at runtime, as the issue itself flagged for that half. CI's Linux jobs will exercise the fallback but not a genuinely reflinked file.