(remote): bound a sync cycle by file count and total bytes - #238
Merged
Conversation
Per-file and per-command bounds already existed, but nothing capped a whole cycle: a host back after a long outage could rebuild its entire mirror at once. Cap toFetch by MAX_CYCLE_FILES and MAX_CYCLE_BYTES, deferring the rest to the next cycle; the existing manifest logic already retries a deferred file correctly, and the deletion pass is untouched since deferral never counts as a fetch failure.
The declared host holds 256 transcripts for 216 MB, so a ceiling of 100 files turned every cold rebuild into three refresh cycles. At 500 the byte ceiling is the one that binds, which is what actually protects the link: 300 files of 60 MB apiece still stop after four.
This was referenced Sep 10, 2026
Merged
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.
Closes #217.
Per-file and per-command bounds already existed — 10 s connect, 60 s inventory,
120 s per scp, 8 MB inventory output, 20 000 inventory entries, 64 MB per file.
Nothing bounded a cycle: neither the number of files nor the total bytes.
A host unreachable for a while then coming back pulled everything at once, four
scp in parallel until done. That scenario is not hypothetical here — the
declared host went unreachable three times in two and a half hours last night.
The ceiling
Two constants in
remote-mirror.js, applied while the fetch list is built:Once either would be exceeded, every remaining eligible entry is deferred and
the cycle stops selecting — a sticky flag, so a smaller file is never
back-filled into the gap ahead of one already deferred. The deferral is logged
once per cycle, in the same style as the existing oversized-file warning.
The two values are sized against the real inventory: the declared host holds
256 transcripts for 216 MB, so its cold rebuild fits in one cycle and the byte
ceiling is the one that binds. A pathological host — 300 files of 60 MB — still
stops after four.
MAX_CYCLE_BYTESis deliberately larger thanMAX_FILE_BYTESso a lone oversized-but-eligible file always fits an otherwise empty cycle: that
is what rules out starvation, without depending on the remote inventory order
being stable between runs.
What was verified rather than rewritten
The deferral mechanism already existed and is untouched: a deferred file keeps
its previous manifest entry, or stays absent if it is new, so the next cycle
still sees it as changed. That is now pinned by a test rather than asserted —
a mirrored file is changed remotely, starved out of a cycle behind fillers,
confirmed untouched on disk, then confirmed to land once room frees up.
The deletion pass is driven by the remote inventory, which stays complete
whatever the fetch ceiling does, so a deferred cycle deletes exactly what an
unbounded one would. Given PR #224 shipped a regression where a failed
inventory silently deleted every mirrored file for a host, this is pinned by
its own test.
Mutation proofs
deletion-under-ceiling tests go red, and only those.
nextFiles[rel] = prevchanged to= meta→ only therefetch-after-deferral test goes red. That is the realistic desync bug.
"deletions still run on a cycle that hits the ceiling" goes red.
task check: 1157 passed, 0 failed, 8 pre-existing skips, lint 0 errors.