Skip to content

fix(tail): finish framer on shutdown to emit trailing partial - #293

Open
detail-app[bot] wants to merge 3 commits into
masterfrom
detail/bug-fix/fix-tail-finish-framer-on-shutdown-to-emit-trailin-43805c
Open

detail-app[bot] wants to merge 3 commits into
masterfrom
detail/bug-fix/fix-tail-finish-framer-on-shutdown-to-emit-trailin-43805c

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Bug

The file-tail runtime (runFilesLoopBackend in src/tail/runtime.zig) never called framer.finish() on shutdown. The watcher sizes events to the file's raw stat size (not a newline boundary), so a trailing record without a \n stays buffered in the framer's scratch after the last event. Without finish(), defer framer.deinit() freed that scratch and silently dropped the partial — permanently lost in the default .tail and .checkpoint modes, where the offset/checkpoint advances to the raw size so the dropped bytes are never re-read on restart (.head re-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

  • Added Runtime.drainFramer, called on the uncanceled shutdown thread after checkpoint.finalize() and before output.flush(). It calls framer.finish() scoped to read_from != .head.head re-reads the whole file from 0 on restart, so emitting at shutdown would duplicate; .tail/.checkpoint never re-read the partial. A catch {} swallows a trailing half-record's parse error under .json/.logfmt so output.flush() still runs (a valid trailing record emits normally).
  • Moved 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

  • Unit tests drive the real Watcher + EngineScheduler + drainFramer: .tail default mode emits an appended partial "part" and a simulated restart neither re-reads nor duplicates it; .checkpoint emits "done\npart"; the .head guard leaves the partial buffered (scratch unchanged); a .json half-record's parse error is swallowed without breaking the flush. Reverting the core fix fails exactly the .tail and .checkpoint tests (2 failures), confirming they guard the fix; restored, all pass.
  • End-to-end via the real edge-tail binary (Debug and ReleaseSafe, real SIGTERM/SIGINT): .tail append part → emits part (exit 0, clean shutdown logs); restart → 0 bytes (no re-read, no duplication); .checkpoint done\npart → emits done\npart; .head done\npart → emits done\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, and zig build tail -Doptimize=ReleaseSafe all pass.

Fixes ENG-694


Automatic Fixes PRs can be configured here.

Comment thread src/tail/runtime.zig Outdated
Comment thread src/tail/runtime.zig
Comment thread src/tail/runtime.zig Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved at 42e2ce1

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 .head restart semantics, and improves signal-waiter cleanup. The added tests directly exercise the affected shutdown and restart paths without introducing schema, security, billing, or infrastructure changes.

Notes:

  • This verdict was updated automatically after the outstanding correctness findings were resolved. Macroscope did not re-review the code.

You can add or adjust custom eligibility rules. Learn more.

…r for signal waiter cleanup, drain before checkpoint finalize
Comment thread src/tail/runtime.zig Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant