Skip to content

fix: ReplicationPoller consumes slot if publication is not empty#1919

Open
edgurgel wants to merge 2 commits into
mainfrom
fix/replication-cdc-no-tables
Open

fix: ReplicationPoller consumes slot if publication is not empty#1919
edgurgel wants to merge 2 commits into
mainfrom
fix/replication-cdc-no-tables

Conversation

@edgurgel
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

  • Change ReplicationPoller to only start a replication slot if the publication is not empty.
  • Also track when publication changes from empty to non-empty and vice-versa and react accordingly

The goal here is only to establish the replication slot when absolute necessary

Why do we even keep the ReplicationPoller up if there is no publication? If we don't whenever a postgres_changes join happens it will start it again. While this approach will leave this process up and running until it notices that there are no subscriptions (SubscriptionManager does this) and stop everything.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the Postgres CDC RLS replication poller lifecycle so it reacts to an empty/non-empty publication by pausing/resuming polling and dropping/recreating the replication slot as publication tables come and go, reducing unnecessary WAL work.

Changes:

  • Add Replications.drop_replication_slot/2 and related tests.
  • Teach ReplicationPoller to check publication table OIDs periodically and stop polling / drop the slot when the publication becomes empty, then resume when tables reappear.
  • Add unit + integration coverage for the poller’s empty-publication behavior and slot toggling.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/extensions/postgres_cdc_rls/replication_poller.ex Adds publication OID tracking and slot drop/recreate behavior tied to publication emptiness.
lib/extensions/postgres_cdc_rls/replications.ex Introduces drop_replication_slot/2 helper for removing logical replication slots.
test/realtime/extensions/cdc_rls/replication_poller_test.exs Updates poller tests and adds coverage for “no tables → no polling” and toggling behavior.
test/realtime/extensions/cdc_rls/replications_test.exs Adds unit tests for drop_replication_slot/2.
test/realtime/extensions/cdc_rls/cdc_rls_test.exs Adds integration tests verifying slot toggling when publication/tables are changed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +259 to +263
defp prepare_replication(
%{
backoff: backoff,
conn: conn,
slot_name: slot_name,
Comment on lines +227 to +240
{n, 0} when n > 0 ->
cancel_timer(state.poll_ref)

case Replications.drop_replication_slot(conn, state.slot_name) do
{:error, reason} when reason != :slot_not_found ->
# The slot is a temporary logical replication slot tied to this connection,
# so stopping the process releases it without leaking WAL.
log_error("DropReplicationSlotFailed", reason)
{:stop, {:shutdown, :drop_replication_slot_failed}, state}

_ ->
cancel_timer(state.check_oid_ref)
{:noreply, %{state | oids: new_oids, poll_ref: nil, check_oid_ref: schedule_check_oids()}}
end
{:error, reason} when reason != :slot_not_found ->
# The slot is a temporary logical replication slot tied to this connection,
# so stopping the process releases it without leaking WAL.
log_error("DropReplicationSlotFailed", reason)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing entry in ERROR_CODES.md :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:tableflip:

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 91.351% (-0.3%) from 91.62% — fix/replication-cdc-no-tables into main

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.

5 participants