fix(outputs.parquet): Rotate on the age of the open file - #19559
Conversation
40d151d to
12e2455
Compare
12e2455 to
e6b4a4e
Compare
|
@81reap #19560 is merged and this one now conflicts in parquet_test.go: both branches append tests at the end of the file, so keeping both sides resolves it. Please rebase onto master, and while you're there the description still needs the two corrections from my earlier comment (the mtime does not move until Close with buffered writes, and the stat error was a per-flush reject rather than a re-queue), since it becomes the commit message. |
|
i was mulling over your comments last night and i discovered that Telegraf already has the file rotation semantics for streamable files here (although it may be for logs, i still need to dig a little deeper after work) https://github.com/influxdata/telegraf/blob/master/internal/rotate/file_writer.go i'm also now questioning if this PR is the best way to solve for this. seems like file rotation should be a common module or design pattern that the rest of the application adopts when it needs it. that way we can standardise fixes for the edge cases you and I are calling out across the whole service |
internal/rotate won't fit here. It's an io.WriteCloser that rotates by renaming the file out from under a live byte stream, which only works for a format where any offset is a valid cut point. Parquet isn't one, the footer and metadata are written on Close, so renaming underneath the pqarrow writer leaves one file with no footer and one whose footer describes rows it doesn't have. That's why the plugin closes the writer and opens a new one instead. There is a real shared concern in there (age tracking, the deleted-file case, archive limits) but a common module would have to be about lifecycle policy rather than the byte stream, and I'd rather not hold this fix for that. Happy to keep it narrow: rebase on #19558 once that lands and this one stays a small, correct fix. |
1dea414 to
f206e51
Compare
Currently rotation compared against the file's modification time via os.Stat on every flush. Since plugins write through a buffered writer, file modificaiton time gets updated with every system flush. If the file was removed underneath the agent, then the write would error and re-queue causing a sall in the pipeline. Now telegraf records when the file was opned and rotates on that.
f206e51 to
985b551
Compare
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Summary
Rotation compared against the file's modification time via
os.Staton every flush. Since the plugin writes through a buffered writer, the modification time only moves when the writer actually flushes to disk, so on a quiet stream it can sit unchanged untilCloseand push the rotation out.If the file was removed underneath the agent, the stat failed and the metrics of that flush were rejected.
Now Telegraf records when the file was opened and rotates on that.
Checklist
Related issues