postgres_cdc: add signalling support#4616
Open
josephwoodward wants to merge 5 commits into
Open
Conversation
postgres_cdc: update tests to verify singal event postgres_cdc: wait for messages to be acked before restarting
josephwoodward
marked this pull request as ready for review
July 21, 2026 17:26
Comment on lines
+269
to
+271
| // confirmedLSNFromDB is unchanged from before the snapshot - the | ||
| // triggering signal was never acked, so resuming here redelivers | ||
| // it. The input layer recognizes and drops that redelivery. |
There was a problem hiding this comment.
This comment describes crash-recovery behavior that does not match the implementation, and contradicts the signal-handling path.
It states the signal "was never acked, so resuming here redelivers it. The input layer recognizes and drops that redelivery." But:
- No drop/dedup logic exists.
postgresSignaller.Listenkeeps no record of previously-processed signals, so a redelivered signal row would simply be re-detected and re-StoreSignaled, triggering another re-snapshot. There is nothing that "recognizes and drops that redelivery." - It contradicts the ack path. The signal handler explicitly acks the signal's LSN before restart precisely so it is never redelivered — see
input_pg_stream.go#L634-L639: "its LSN is durably confirmed to Postgres before we restart - it will never be redelivered, so there's no redelivery loop to guard against."
Please reconcile the two: if the signal is durably acked (as input_pg_stream.go does), then confirmedLSNFromDB here is not "unchanged" and this redelivery reasoning is misleading. If the signal can still redeliver (e.g. a crash before the standby status update reaches Postgres), the "recognizes and drops" mechanism the comment relies on needs to actually exist, otherwise the redelivered signal re-triggers a snapshot.
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.
This change introduces signalling support into the PostgreSQL CDC connector allowing you to perform operations such as signal a new snapshot on an adhoc basis. It does this via a custom created signal table which is also being tracked by PostgreSQL's replication functionality, with control signals propagating from the WAL. When a signal is detected the signaller we determine the signal type then handle it accordingly. Currently only snapshots are supported but other signal types can added over time (such as logging).
Upon receiving a signal event, the process will:
Demonstration
Given the following config:
In the screenshot below the process goes:
benchmark_results.jsontask psql:signal:snapshotto publish a snapshot signal to the configured signalling table which the connector receives, pauses streaming then restarts to snapshot (now 201 records, 100x2 plus 1 signal event)