Fix permanent three-node sync divergence; bound the stress test by rate - #34
Merged
Conversation
The fixed counts were duration-sensitive, which made this test flaky rather than strict. The writers perform a fixed NUMBER of revisions with a sleep between them, so a loaded machine stretches the burst, elapses more coalescing windows, and does proportionally more legitimate work. Measured: it passes locally at 30 to 34 reconciles against an old cap of 36, a margin of two, while Linux CI produced 54 in 15.57s and 43 in 16.14s. Every one of those runs, both failures included, stayed under 4 reconciles per second. So the old caps were failing runs whose actual amplification was fine, and raising them would only move the cliff. The ceilings keep the original relationships exactly: the reconcile rate stays at 4 per second and the scan and persist ceilings are the old caps expressed against that same rate, 112/36 and 76/36 of it, which is the derivation the previous comment already described. No new number is invented. Validated against all four observed runs including both failures. This is the mode-1 half of the review. The engine defect it was masking is next.
Fixes the permanent three-node divergence. The cause was mine, from two commits earlier in this line: ccddbee added a scan cache that reuses a recorded hash when size and both mtime components match, and a5bfde2 made materialization stamp the ORIGIN's mtime so that cache would hit on materialized files. Together they turned mtime from a local write timestamp into a value shared across nodes, which is the assumption the cache cannot survive. Two nodes contending on one key produce entries of equal size that can carry equal mtimes with different content. The scan then reuses the recorded hash and reports content the file does not hold; the republish branch reads the real bytes, sees a different hash, concludes a user edited the file, and republishes stale content under local authorship. Both sides do it and versions leapfrog forever. Measured on Linux CI run 30814462024 attempt 4: twenty-four consecutive republishes alternating between two nodes, every line showing the prior manifest hash equal to the scanned hash yet a different hash afterwards, v44 through v67, never converging. journal_match and journal_committed were true throughout, which is what ruled out the daemon-write journal and pointed here instead. A materialized file now carries the local write time, so the cache misses on it and the next scan reads the real bytes. Untouched local files still hit the cache, which is where the benefit actually was. The cost is one re-read per materialized file, asserted directly rather than described. Proofs: materialization no longer manufactures an mtime collision; an untouched file still gets a cache hit and is not re-read; a real user edit still republishes with local authorship and a higher version. The collision test also asserts the residual risk this does NOT close, that the cache still trusts a collision arriving by some other route, since closing that needs a content-identity mechanism that was deliberately deferred. The test that previously asserted the stamping worked now asserts the opposite and carries the reason, because that assertion was the defect. Carries the mode-1 rate correction in the same review.
This was referenced Aug 3, 2026
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.
Two commits, one review.
Engine fix. Materialization no longer stamps the origin's mtime. That stamping (mine,
a5bfde2) existed so the scan cache (ccddbee, also mine) would hit on materialized files, but it made mtime a value shared across nodes — the one assumption that cache cannot survive. Two nodes contending on a key produce equal-size entries that can collide on size and mtime with different content; the scan then reports bytes the file does not hold, the republish branch reads the real bytes, and stale content is republished under local authorship. Both sides do it and versions leapfrog forever.Evidence, Linux CI run
30814462024attempt 4: twenty-four consecutive alternating republishes, v44→v67, never converging, every line with prior manifest hash equal to the scanned hash but a different hash after.journal_match/journal_committedtrue throughout, which ruled out the daemon-write journal.Cost, asserted rather than described: one re-read per materialized file. Untouched local files still hit the cache.
Mode-1 test correction. The continuous-mutation stress is now bounded by rate, not fixed counts, preserving the original relationships exactly (4 reconciles/s, scans and persists at 112/36 and 76/36 of it). Validated against all four observed runs including both failures.
Residual risk asserted, not hidden: the cache still trusts a size+mtime collision arriving by another route. Closing that needs a content-identity mechanism, deliberately deferred.