Skip to content

fix(transport/file): write record and separator atomically - #526

Open
dmcken wants to merge 1 commit into
netsampler:mainfrom
dmcken:fix/atomic-file-transport-write
Open

fix(transport/file): write record and separator atomically#526
dmcken wants to merge 1 commit into
netsampler:mainfrom
dmcken:fix/atomic-file-transport-write

Conversation

@dmcken

@dmcken dmcken commented Aug 29, 2026

Copy link
Copy Markdown

So I've been tracking an issue when I point production traffic to a goflow2 server writing protobuf directly to disk (double-writes with Kafka will speed up the death of my SSDs). To give an idea of my traffic levels, the protobuf file for 15 minutes is about 2.1GB (at peak to be fair). I've been tracking an issue where the files get corrupted when traffic levels climb. I was performing the analysis / reading in rust which I was learning at the time (https://github.com/dmcken/goflow2_reader_rs) so I honestly for quite a while thought the issue was my code. With a hex editor I could find end of lines in the protobuf file in the wrong locations which gave the initial hints as to what was going on (the records all fall within a certain range of lengths).

Weirdly enough JSON was fine, so as a temporary fix I was processing JSON from /dev/shm (to avoid massacring my disks) and then turning that into a binary format, hence the gaps before this PR. As Go isn't a language I'm that familiar with I've been letting Claude analyze a running server for the past week and it found the below issue after confirming I wasn't going crazy. Another fix that worked was dropping to workers=1 which removed the concurrency problem (but creates a max throughput problem, with this fix in place I can scale to multiple workers).

At this point I'm not sure what the AI policy of this project is, this PR was written with the help of Claude. I can't easily post corrupted files here as I can't anonymize them without being able to read them consistently. Let me know of any issues / tests you want to see and I'll try to accommodate to my best ability, the fix looks pretty simple so if its just simpler for a maintainer to re-write it I'm perfectly fine with that (probably better written by someone who knows Go better than I do), I just want a goflow2 that can handle the load I'm throwing at it.

Claude summary of issue and fix:
Send() issued two separate Write() calls (data, then separator). With more than one decode worker calling Send() concurrently (collector's workers listen option), another goroutine's write can land between them, dropping the separator for that record and merging it with the next one on disk.

Observed in production: a file transport running with workers=16 produced records where a fully valid new FlowMessage started at the exact byte offset a separator should have been, corrupting length-prefixed framing for any binary-format consumer built on top of it. Reducing to workers=1 eliminated the corruption immediately (confirmed clean across 1.3M+ decoded records), which pointed at a write race rather than a decode/encode bug.

Fix: build the record+separator into one buffer and issue a single Write() call, which is atomic with respect to other writers on a regular file opened O_APPEND. No behavior change when a separator isn't configured.

Send() issued two separate Write() calls (data, then separator). With
more than one decode worker calling Send() concurrently (collector's
`workers` listen option), another goroutine's write can land between
them, dropping the separator for that record and merging it with the
next one on disk.

Observed in production: a file transport running with workers=16
produced records where a fully valid new FlowMessage started at the
exact byte offset a separator should have been, corrupting
length-prefixed framing for any binary-format consumer built on top
of it. Reducing to workers=1 eliminated the corruption immediately
(confirmed clean across 1.3M+ decoded records), which pointed at a
write race rather than a decode/encode bug.

Fix: build the record+separator into one buffer and issue a single
Write() call, which is atomic with respect to other writers on a
regular file opened O_APPEND. No behavior change when a separator
isn't configured.
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.

1 participant