RE: Improve pg_sync_replication_slots() to wait for primary to advance

On Friday, January 30, 2026 5:49 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
>
> Thanks for the patch. Few comments:

Thanks for the comments.

>
> 1)
> * If the SQL function pg_sync_replication_slots() is used to sync the slots,
> * and if the slots are not ready to be synced and are marked as
> RS_TEMPORARY
> * because of any of the reasons mentioned above, then the SQL function also
> * waits and retries until the slots are marked as RS_PERSISTENT (which
> means
> * sync-ready). Refer to the comments in SyncReplicationSlots() for more
> * details.
>
> We need to update the comment at the top of the file as well.

Updated.

>
> 2)
> Is there a reason we don't call should_resync_slot() in
> synchronize_slots() after each synchronize_one_slot() call? If we did, we could
> invoke it in a single place instead of calling it at multiple locations as we do
> now.

Since should_resync_slot() can only be invoked when the slot is acquired, it
cannot be called in synchronize_slots() where slot has been released. I thought
of searching for that slot again, but that seems to bring unnecessary cost and
codes.

>
> 3)
> In commit message, we have mentioned pg_sync_replication_slot() at one
> palce instead of pg_sync_replication_slots().

Updated.

>
> 4)
> In the test, can you please elaborate why we need
> 'synchronized_standby_slots' adjustment twice?
>
> # Remove the standby from the synchronized_standby_slots list and reload
> the # configuration.
> $primary->adjust_conf('postgresql.conf', 'synchronized_standby_slots', "''");
> $primary->reload;
>
> # Remove the standby from the synchronized_standby_slots and reduce the
> maximum # walsender number.
> $primary->append_conf(
> 'postgresql.conf', qq(
> max_wal_senders = 2
> synchronized_standby_slots = ''
> ));
> $primary->restart;

Sorry, it's a copy-paster error, removed one of them.

>
> 5)
> At the end of the test, where do we check that the API is no longer continuing
> and has successfully finished? I see '$h->quit;', but is that equivalent to
> verifying that the API is successfully over?

The test first confirms that slot_skip_reason is set to NULL,
indicating successful synchronization, and then confirms that the backend has exited.
(If the function is blocking, the `$h->quit;` cannot pass.)

Here are the updated patches.

Best Regards,
Hou zj