On Friday, January 30, 2026 5:49 PM shveta malik <[email protected]> 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
v2-0001-Improve-the-retry-logic-in-pg_sync_replication_sl.patch
Description: v2-0001-Improve-the-retry-logic-in-pg_sync_replication_sl.patch
v2-0002-Add-a-taptest.patch
Description: v2-0002-Add-a-taptest.patch
