pgremote: follow the log instead of stopping at its current end - #6
Merged
Conversation
Open returned a reader that ended at the last byte the server had written when it was called. That is right for a one-shot read and wrong for a log reader: pgwatch's consume returned, parseStream sent one measurement and returned, and runLogParser read that clean return as success and never called again. Log parsing reported once per pgwatch start and then went silent, while every other metric kept flowing -- a failure with no error anywhere to notice it by. FileSet had Follow and PollInterval for the local path all along. This adds them to the remote one, so a caller can switch between the two readers without one of them quietly being a different kind of reader. Following needs somewhere to record progress: a follower re-lists the directory on every poll and decides what is new from the stored offsets, so with Config.Offsets nil it would deliver the whole directory again on every pass. Open now substitutes a bounded in-memory store when Follow is set, which is what makes the flag mean what it says. Two cases follow mode has to get right that a one-shot reader does not: Reaching the end of what the server has written is not reaching the end of the file. A trailing line with no newline is the file's last line to a one-shot reader (FMT-009) and a record still being written to a follower. Flushing it would split one log record into two, each half parsing as malformed, and the event it describes counted zero times or twice. A follower rewinds past the partial line and drops it; the recorded offset points at its first byte and the next poll re-reads it whole. Cancellation is an error to a reader that was going to finish on its own and the designed stop signal to one that never will. A cancelled follower now ends at io.EOF like any exhausted reader, matching FileSet, rather than making every caller special-case a normal shutdown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Trailing comments on two adjacent short-variable declarations were aligned by hand, which gofmt undoes. Caught by the pgremote lint job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
What
Adds
FollowandPollIntervaltopgremote.Config, so the remote reader keeps reading instead of ending at the last byte the server had written whenOpenwas called.FileSethas had both for the local path all along. The remote path did not, and nothing in either API said so.Why
Found by running the pgwatch docker-compose deployment against three PostgreSQL instances, one per
log_destination.server_log_event_countswas written once per source and then never again, while every other metric kept flowing:server_log_event_countsdb_stats(control)The csvlog file grew 318 KB → 949 KB in that window, all of it unread.
The chain:
pgremotereturnsio.EOFonce it drains the files → pgwatch'sconsumereturns →parseStreamsends one measurement and returnsnil→runLogParsertreats a clean return as success and never calls again. No error is logged anywhere, so the only symptom is a metric that stops.This is a regression from the pglogwatch migration — the pre-migration
parseLogsRemotehad an infinite poll loop overpg_ls_logdir. Nothing replaced it. The local path was unaffected becauseopenLocalalready passedFollow: true.Two cases follow mode has to get right
A half-written line is not a record. Reaching the end of what the server has written is not reaching the end of the file. A trailing line with no newline is the file's last line to a one-shot reader (FMT-009), and a record still being written to a follower. Flushing it would split one log record in two — half delivered now, half prepended to the next poll's chunk — with both halves parsing as malformed and the event counted zero times or twice. A follower rewinds past the partial line and drops it; the recorded offset points at its first byte, so the next poll re-reads it whole.
Cancellation means different things to the two readers. A one-shot reader was going to finish on its own, so being cancelled cost the caller data and is an error. A follower never finishes on its own — cancelling it is the only way to stop it. A cancelled follower now ends at
io.EOF, matchingFileSet, rather than making every caller special-case the normal shutdown path.Note for reviewers
Config.Offsetsis documented as "nil means no persistence". For a follower that would mean re-delivering every file on every poll, since a follower re-lists the directory and decides what is new from the stored offsets.Opennow substitutes a bounded in-memory store whenFollowis set. Behaviour withFollowfalse is unchanged.relistdeliberately does nothing but replace the file slice and rewind the index — every decision about what to read next already lives innextFile, which compares each listed size against the stored offset. The first pass and the follow path take the same decisions from the same evidence.Testing
pgremote/follow_test.go, asserting on the reader's bytes rather than parsed records — what follow mode changes is framing, and a test that only counted records would pass while the counts it produced were wronggo test -race ./...green in this module and the root module; existing tests unchangedFollow-up
Needs a
pgremote/v1.0.1tag; pgwatch currently carries a temporaryreplacepointing here.🤖 Generated with Claude Code