fix(tail): finish framer on shutdown to emit trailing partial - #293
Open
detail-app[bot] wants to merge 3 commits into
Open
detail-app[bot] wants to merge 3 commits into
detail-app[bot] wants to merge 3 commits into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a contained tail-runtime bug fix that emits previously dropped trailing partial records during clean shutdown, preserves Notes:
You can add or adjust custom eligibility rules. Learn more. |
…r for signal waiter cleanup, drain before checkpoint finalize
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.
Detail bug report: View on Detail
Bug
The file-tail runtime (
runFilesLoopBackendinsrc/tail/runtime.zig) never calledframer.finish()on shutdown. The watcher sizes events to the file's raw stat size (not a newline boundary), so a trailing record without a\nstays buffered in the framer's scratch after the last event. Withoutfinish(),defer framer.deinit()freed that scratch and silently dropped the partial — permanently lost in the default.tailand.checkpointmodes, where the offset/checkpoint advances to the raw size so the dropped bytes are never re-read on restart (.headre-reads from 0 on restart, so it recovered).A second issue blocked the fix from taking effect on real signal shutdown: the worker threads (checkpoint lane + poll loop) were spawned before the SIGINT/SIGTERM signal mask was set, so a SIGTERM landed on a worker by default action and terminated the process before the sigwait thread could drive the clean shutdown drain.
Fix
Runtime.drainFramer, called on the uncanceled shutdown thread aftercheckpoint.finalize()and beforeoutput.flush(). It callsframer.finish()scoped toread_from != .head—.headre-reads the whole file from 0 on restart, so emitting at shutdown would duplicate;.tail/.checkpointnever re-read the partial. Acatch {}swallows a trailing half-record's parse error under.json/.logfmtsooutput.flush()still runs (a valid trailing record emits normally).installSignalWaiter(which blocks the signal set and spawns the sigwait thread) to run before spawning the checkpoint worker and poll loop, so those threads inherit the blocked mask and only the sigwait thread receives signals — making the clean shutdown drain reachable on SIGTERM/SIGINT.Testing
Watcher+EngineScheduler+drainFramer:.taildefault mode emits an appended partial"part"and a simulated restart neither re-reads nor duplicates it;.checkpointemits"done\npart"; the.headguard leaves the partial buffered (scratch unchanged); a.jsonhalf-record's parse error is swallowed without breaking the flush. Reverting the core fix fails exactly the.tailand.checkpointtests (2 failures), confirming they guard the fix; restored, all pass.edge-tailbinary (Debug and ReleaseSafe, real SIGTERM/SIGINT):.tailappendpart→ emitspart(exit 0, clean shutdown logs); restart → 0 bytes (no re-read, no duplication);.checkpointdone\npart→ emitsdone\npart;.headdone\npart→ emitsdone\n(guard, recovered on restart); newline-terminated input unaffected; rotation/truncation recovery (offset reset to 0, re-read) intact. Pre-fix, SIGTERM exited 143 (killed by default action) with no drain; post-fix it exits 0.zig fmt --check,ziglint,zig build test, andzig build tail -Doptimize=ReleaseSafeall pass.Fixes ENG-694
Automatic Fixes PRs can be configured here.