Key the scan cache locally, and replicate the executable bit - #47
Merged
Conversation
Two changes with one theme: fabric should agree with git about what a catalog is, and a local cache must never take its key from a replicated structure. THE CACHE. scan_folder reused a recorded hash when size and mtime matched what the MANIFEST held. The manifest crosses the wire, so a local caching decision was made from a value another machine chose. Two contending entries of equal size could collide on size plus mtime, the cache reported content the file did not hold, and the versions leapfrogged forever. That was the permanent three-node divergence, and removing mtime stamping only removed the mechanism that manufactured the collision, not the conflation that believed it. A separate scan_cache now records what THIS machine observed on its own disk, and never crosses the wire. It is deliberately NOT merged into the observed receipt: that receipt decides whether a missing path becomes a tombstone, and a tombstone is sent to peers, so loading a performance concern onto it would repeat the original mistake mirrored. Two structures, two jobs. It is filled only from real disk reads, never from a requested value, so a filesystem that truncates a timestamp cannot make it miss forever. Absent in an older state file, where serde default gives an empty one that warms on the first scan. THE EXECUTABLE BIT. Fabric tracked content and nothing else. Git tracks content, the executable bit and symlinks, and deliberately not mtime. The synced catalog holds fabric binaries, and they arrived without the bit and could not be run until somebody chmod-ed them by hand. The bit is now recorded and applied before the atomic rename, so a file is never briefly visible with the wrong mode. FileMeta.mtime is demoted rather than removed. It is documented as informational and is never applied. Removing it would break parsing on any peer not yet upgraded, because the field has no serde default; adding a field with a default is safe in a way removing one is not. Symlinks are still skipped, but they now say so and say that git tracks them. Silence was the problem. ONE CAUSE, TWO SYMPTOMS, NAMED IN ONE PLACE. local_write returns early when the content hash is unchanged, which is what makes applying a peer's content echo-free. So no change that alters zero bytes can propagate. An invisible heartbeat and an invisible chmod are the same defect, not two. Both symptoms are pinned by tests that assert they still do NOT propagate, so nobody fixes one and assumes the other followed, and the chmod case is recorded as a DIVERGENCE FROM GIT rather than an implementation note. Verified by sabotage, not only by green: dropping mtime_nanos from the cache key fails the sub-second test AND the three-node stress; never applying the mode fails the executable test. 211 lib tests green, plus lifecycle, provisioning and the real-iroh sync slice.
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
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.
Approved by cos after Nathan's rule: fabric should sync the same file attributes git does, and a catalog should be carriable by git or fabric — so the two must agree about what a catalog is.
The cache
scan_folderreused a recorded hash when size and mtime matched what the manifest held. The manifest crosses the wire, so a local caching decision was made from a value another machine chose. Two contending entries of equal size could collide on size + mtime, the cache reported content the file did not hold, and versions leapfrogged forever — the permanent three-node divergence.Removing mtime stamping (62a30b8) removed the mechanism that manufactured the collision. It did not remove the conflation that believed it.
A separate
scan_cachenow records what this machine observed on its own disk and never crosses the wire.It is deliberately not merged into
observed. That receipt decides whether a missing path becomes a tombstone, and tombstones are sent to peers — so loading a performance concern onto it would repeat the original mistake mirrored. cos authorized wideningobserved; I disagreed and they accepted. Two structures, two jobs.It is filled only from real disk reads, never from a requested value, so a filesystem that truncates a timestamp cannot make it miss forever. Absent in an older state file, where
serde(default)gives an empty one that warms on the first scan.The executable bit
The live case: the synced catalog holds fabric binaries, and they arrived without the bit and could not be run until somebody chmod-ed them by hand. The bit is applied before the atomic rename, so a file is never briefly visible with the wrong mode.
FileMeta.mtimeis demoted, not removed. Removing it would break parsing on any peer not yet upgraded — the field has no serde default, and droppy/hetz are not on this build. Adding a field with a default is safe in a way removing one is not.One cause, two symptoms — named in one place
local_writereturns early when the content hash is unchanged, and that early return is what makes applying a peer's content echo-free. So no change that alters zero bytes can propagate:These are the same defect, not two. Both are pinned by tests asserting they still do not propagate, so nobody fixes one and assumes the other followed. The chmod case is recorded as a divergence from git — git propagates a chmod, because a mode change rewrites the tree object.
Verified by sabotage, not only by green
mtime_nanosfrom the cache key → fails the sub-second test and the three-node stressThe sub-second test exists because an earlier sabotage run showed the original tests did not catch a weakened key.
Tests
211 lib green, plus lifecycle, provisioning, and the real-iroh sync slice.