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

>
> I have addressed the above comments in patch v21.
>

Thank You. Please find a few comments:

1)
+ fparams.slot_names = slot_names = NIL;

I think it is not needed to set slot_names to NIL.

2)
- WAIT_EVENT_REPLICATION_SLOTSYNC_MAIN);
+ WAIT_EVENT_REPLICATION_SLOTSYNC_PRIMARY_CATCHUP);

The new name does not seem appropriate. For the slotsync-worker case,
even when the primary is not behind, the worker still waits but it is
not waiting for primary to catch-up. I could not find a better name
except the original one 'WAIT_EVENT_REPLICATION_SLOTSYNC_MAIN'. We can
change the explanation to :

"Waiting in main loop of slot sync worker and slot sync API."
Or
"Waiting in main loop of slot synchronization."

If anyone has any better name suggestions, we can consider changing.

3)

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

Shall we remove 'with the locally reserved position', it’s already
explained in the test header and the comment is good enough even
without it.

4)
+# Confirm log that the slot has been synced after becoming sync-ready.

Shall we just say:
Confirm from the log that the slot is sync-ready now.

5)
# Synchronize the primary server slots to the standby.
$standby1->safe_psql('postgres', "SELECT pg_sync_replication_slots();");
@@ -945,6 +1007,7 @@ $subscriber1->safe_psql('postgres',
is( $standby1->safe_psql(
'postgres',
q{SELECT count(*) = 2 FROM pg_replication_slots WHERE slot_name IN
('lsub1_slot', 'snap_test_slot') AND synced AND NOT temporary;}
+
),

Redundant change.

thanks
Shveta