Repository navigation
A followed file is resumed where the store ends in it, not by its first stamp - #202
Merged
Merged
Conversation
…st stamp On start, catch_up compared the file's first timestamp with the store's first chunk, and a file that began before the store skipped entirely and the tail started at its end. Once retention has dropped the store's head, a long-lived file always begins before it, so a restart (an upgrade, a reboot, a crash) lost every line written while it was down. catch_up now first asks where the store's data ends in the file, by comparing the store's last bytes with the file's before that offset (the byte-verified resume import already uses, now resume_point), and carries on from there with no scan and no hash set of the store. It counts bytes dropped by retention, so it holds for a trimmed store, and it never errors on an ambiguity: where it cannot tell, the existing line-by-line re-sync runs as before. A rotated file is finished the same way and its trailing fragment committed. A store with retention, a 6,000-line file, a restart after 100 lines were written while down: 0 of them arrived before, 100 after. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.
Fixes data loss in the live importer (
import --follow,file-intake) on restart.Problem
On start,
catch_upcompared the file's first timestamp with the store's first chunk. A file that began before the store was skipped entirely, and the tail started at its end:Once retention has dropped the store's head, a long-lived file always begins before it. So a restart (a package upgrade through
--exit-on-upgrade, a reboot, a crash) of a follower on a store with retention loses every line written while it was down. A running follower is not affected, only a restarted one.Reproduced end to end with the released binary: a 6,000-line file followed into a store, retention dropped the head while the follower was stopped, 100 lines written to the file meanwhile, follower restarted: 0 of the 100 reached the store.
Change
catch_upfirst asks where the store's data ends in the file, by bytes: the store's last 16 KiB against the file's bytes just before that offset (the byte-verified resumeimportalready has since #201, now one function,import::resume_point). If it verifies, the tail carries on from there: no scan of the file and no hash set of the store. It uses the tape end (dropped + size), so it holds for a store whose head retention dropped, which is exactly what the first-stamp comparison got wrong.claims_same = false), because underRestart=alwaysan exit is a restart loop. Where it cannot tell, the existing line-by-line re-sync runs as before..1,.0) is finished the same way and its trailing fragment committed.import's behaviour is unchanged; the refactor only lifts itsmatchinto a function.Checked with the real binaries
Same scenario,
mainagainst this branch:skipping app.log — it starts 12:00:00, before the oldest data in s.logs.log ends at byte 574890 of it (579890 bytes); resuming thereTests
Five in
follow.rs, each simulating a process start exactly ascmd_followdoes it (open the store,catch_upthe rotated files and then the live one, drain): a restart after retention dropped the head picks up what was written while down; a restart with nothing new changes nothing; a line still being written when it stopped arrives whole; a file rotated while down is finished and then the new one is read (with a trimmed store); a file that does not continue the store is re-synced line by line as before. With the new resume disabled, exactly the two data-loss tests fail and the other three still pass.The man page, the deployment guide and the two unit files described the startup scan as the normal case; they now say it is the fallback.
Not in this PR, and a remaining hazard
catch_uptakes the old path, wheret0 < firststill skips the file and starts at its end. Stage 2 (the hint file, with anchors) narrows when that happens, and the plan's failure policy says what the fallback should do instead; I did not change it here..1/.0names only.🤖 Generated with Claude Code