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

On Fri, Nov 28, 2025 at 10:16 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
>
> Fixed as above.
>
> I've addressed the above comments as well as rebased the patch based
> on changes in commit 76b7872 in patch v26
>

Thanks for the patch. Please find a few trivial comments:

1)
+ if (AmLogicalSlotSyncWorkerProcess())
+ Assert(sync_replication_slots);

Here too we can use 'worker'.

2)
+ /* check for sync_replication_slots change */

check --> Check

3)
Assert (!worker)
Extra space in between.

4)
check_and_set_sync_info() and ShutDownSlotSync() refers to the pid as
worker_pid. But now it could be backend-pid as well.
Using 'worker' in this variable could be misleading. Shall we make it
sync_process_pid?

5)
/*
* Interrupt handler for main loop of slot sync worker.
*/
static void
ProcessSlotSyncInterrupts()

We can modify the comment to include API as well.

6)
/*
* Shut down the slot sync worker.
*
* This function sends signal to shutdown slot sync worker, if required. It
* also waits till the slot sync worker has exited or
* pg_sync_replication_slots() has finished.
*/
void
ShutDownSlotSync(void)

We should change comments to give details on API as well.

7)
+# Remove the standby from the synchronized_standby_slots list and reload the
+# configuration.
+$primary->adjust_conf('postgresql.conf', 'synchronized_standby_slots', "''");
+$primary->reload;

We can update the comment to below for better clarity:
Remove the dropped sb1_slot from the ...

8)
+# Attempt to synchronize slots using API. The API will continue retrying
+# synchronization until the remote slot catches up.
+# The API will not return until this happens, to be able to make
+# further calls, call the API in a background process.

We can move these comments atop:
my $h = $standby2->background_psql('postgres', on_error_stop => 0);

thanks
Shveta