Skip to content

fix(filetail): distinguish append growth from replacement - #161

Merged
jmagar merged 1 commit into
mainfrom
fix/filetail-start-at-end-race
Aug 2, 2026
Merged

fix(filetail): distinguish append growth from replacement#161
jmagar merged 1 commit into
mainfrom
fix/filetail-start-at-end-race

Conversation

@jmagar

@jmagar jmagar commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

Post-merge Tests for aed693be failed in run 30748902551, job 91499296468:

filetail::supervisor_tests::reconcile_initializes_start_at_end_checkpoint_before_returning
left:  "already here"
right: "after reconcile"

The checkpoint handoff was valid: the stored and opened file identity matched and the initial offset was 13. The reader later misclassified normal append-only growth as same-inode replacement.

For files shorter than the 256-byte fingerprint window, the baseline fingerprint contains the whole original file. If an append lands between EOF detection and the rotation check, the new fingerprint is longer, so the code treats the append as replacement and reopens at byte 0.

Fix

  • read a full current fingerprint but compare only against the stored baseline prefix
  • treat append-only growth as growth rather than same-inode replacement
  • preserve the full current fingerprint when a real replacement is detected
  • preserve rename, copytruncate, and same-inode replacement detection
  • add deterministic regressions for both append growth and full-baseline refresh after replacement

Verification

  • pre-fix 2,000-run / 16-worker stress reproduced the race at run 1026
  • instrumented run reproduced it at run 656 while proving the checkpoint matched
  • reviewed/fixed stress: 2,000/2,000 passed
  • complete file-tail suite: 39 passed, 0 failed
  • reopen/copytruncate regressions: 6 passed, 0 failed
  • all-target check: passed
  • strict Clippy with warnings denied: passed
  • formatting, diff integrity, module-size, pre-commit, and pre-push gates: passed
  • Copilot review finding addressed and resolved

This also unblocks clean post-merge CI so the corrected Release Please workflow from #158 can be verified.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a file-tail rotation heuristic bug where normal append-only growth (especially for files shorter than the 256-byte fingerprint window) could be misclassified as same-inode replacement, causing the tailer to reopen at byte 0 and re-read old data. It adjusts fingerprint comparison logic to distinguish true replacement from ordinary growth and adds a regression test to lock in the correct behavior.

Changes:

  • Parameterize prefix fingerprint reads so the rotation check can compare only the baseline prefix length.
  • Keep using the full 256-byte baseline fingerprint window for fresh opens/reopens while comparing only the stored baseline prefix.
  • Add a deterministic test ensuring short-file append growth does not trigger a reopen-from-zero.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/filetail/supervisor/io.rs Updates the prefix fingerprinting/rotation detection logic to compare only baseline bytes while preserving a stable baseline window.
src/filetail/supervisor_tests.rs Adds a regression test that reproduces and prevents the short-file append misclassification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/filetail/supervisor/io.rs Outdated
@jmagar
jmagar force-pushed the fix/filetail-start-at-end-race branch from d7d9c97 to 87a4eb9 Compare August 2, 2026 14:02
@jmagar
jmagar merged commit 337f608 into main Aug 2, 2026
18 checks passed
@jmagar
jmagar deleted the fix/filetail-start-at-end-race branch August 2, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants