feat(output): add transactional record spool - #50
Open
ronheichman wants to merge 7 commits into
Open
Conversation
Store complete records in a transactional queue and acknowledge them only after successful HTTP delivery. Built with Codex
The failure that corrupted the legacy append file was a write that could not grow its backing file: a short or failed append left a partial NDJSON line that the next record concatenated with. Assert the spool is immune to that trigger by capping RLIMIT_FSIZE in a child process (so the cap cannot disturb the test harness) and forcing a multi-megabyte Put to fail during bbolt's file growth. The store must stay byte-identical to its pre-Put state and remain usable once space is available, so a failed write can never leave a partial or glued record.
A hook appends one NDJSON record per process to the shared records file. On a full disk the kernel wrote what fit in the last block and then failed, leaving a record with no trailing newline; the next hook's record appended onto the same line, and the shipper delivered the glued pair as one line that ingestion rejected as invalid JSON, stalling the queue. Hold the existing file lock across the whole append and keep the file ending on a record boundary: repair a missing trailing newline before writing (append sinks now open read-write so the last byte can be read), and truncate a short or failed write back to the pre-write size. Either way the next record starts on its own line instead of concatenating onto a partial one.
An already-poisoned records file, written before the append-side fix, can still contain a line that is two records glued together by an interrupted append. Shipping it makes ingestion reject the whole batch, and the checkpoint never advances past it, so the queue stalls indefinitely. Skip a complete line that is not a single JSON object the same way an oversized record is skipped: ship any good records buffered before it, advance the checkpoint past it with a diagnostic, and continue. The record stays in the input file for inspection; only its HTTP delivery is skipped. The object check mirrors the spool sink.
spoolSink.Write and isShippableRecord both required a record to be a single JSON object, and the two open-coded the same check. Route the sink through isShippableRecord so the enqueue-time and ship-time validations cannot drift.
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.
A short or disk-full append can leave a partial NDJSON record in the live output file. The next hook append then completes the same line with a different record, so HTTP ingestion rejects the malformed line and the legacy
ship --input-filecheckpoint cannot advance past it — one poisoned line stalls the whole queue.This PR adds a durable, opt-in spool output and, in the same change, hardens the existing legacy file path so already-installed hooks stop producing glued lines and drain past any that predate the fix.
Transactional spool
An opt-in bbolt-backed spool output that:
2xx;ship --input-fileavailable for existing append-only NDJSON deployments.Operators opt in with
--output spool --spool-file PATHand runnumbat ship --spool-file PATH --http-url URLseparately. Delivery remains at least once, so receivers must tolerate duplicate record IDs after ambiguous HTTP outcomes. Existing hook installs continue to default to file output and are not migrated automatically.Legacy file path hardening
The file sink now keeps the records file ending on a record boundary. Under the same file lock it holds today, it repairs a missing trailing newline before writing (append sinks open read-write so the last byte can be read) and truncates a short or failed write back to the pre-write size. Either way the next record starts on its own line instead of concatenating onto a partial one.
ship --input-filenow skips a complete line that is not a single JSON object, the same way it already skips an oversized record: it ships any good records buffered before the bad line, advances the checkpoint past it with a stderr diagnostic, and continues. The line stays in the input file for inspection; only its HTTP delivery is skipped. This drains files that were already poisoned before the append-side fix.Tests
RLIMIT_FSIZEcap in a child process) is rolled back so the next record does not glue onto it.Put(sameRLIMIT_FSIZEtechnique) commits nothing and the store stays byte-identical and usable.