Re: Synchronizing slots from primary to standby

[Amit Kapila]

On Mon, Mar 4, 2024 at 9:35 AM Peter Smith <smithpb2...@gmail.com> wrote:
>
> OK, if the code will remain as-is wouldn't it be better to anyway
> change the function name to indicate what it really does?
>
> e.g.  NeedToWaitForWal --> NeedToWaitForWalFlushOrStandbys
>

This seems too long. I would prefer the current name NeedToWaitForWal
as waiting for WAL means waiting to flush the WAL and waiting to
replicate it to standby. On similar lines, the variable name
standby_slot_oldest_flush_lsn looks too long. How about
ss_oldest_flush_lsn (where ss indicates standy_slots)?

Apart from this, I have made minor modifications in the attached.

-- 
With Regards,
Amit Kapila.

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index aa477015bd..1530e7720c 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2390,8 +2390,8 @@ validate_standby_slots(char **newval)
        {
                /*
                 * We cannot validate the replication slot if the replication 
slots'
-                * data has not been initialized. This is ok as we will 
validate the
-                * specified slot when waiting for them to catch up. See
+                * data has not been initialized. This is ok as we will anyway 
validate
+                * the specified slot when waiting for them to catch up. See
                 * StandbySlotsHaveCaughtup for details.
                 */
        }
@@ -2473,8 +2473,7 @@ assign_standby_slot_names(const char *newval, void *extra)
        char       *standby_slot_names_cpy = extra;
 
        /*
-        * The standby slots may have changed, so we need to recompute the 
oldest
-        * LSN.
+        * The standby slots may have changed, so we must recompute the oldest 
LSN.
         */
        standby_slot_oldest_flush_lsn = InvalidXLogRecPtr;
 
@@ -2664,6 +2663,7 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int 
elevel)
        if (caught_up_slot_num != list_length(standby_slot_names_list))
                return false;
 
+       /* The standby_slot_oldest_flush_lsn must not retreat. */
        Assert(XLogRecPtrIsInvalid(standby_slot_oldest_flush_lsn) ||
                   min_restart_lsn >= standby_slot_oldest_flush_lsn);
 
diff --git a/src/backend/replication/walsender.c 
b/src/backend/replication/walsender.c
index b71408d533..580f9dabd3 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1805,10 +1805,10 @@ NeedToWaitForWal(XLogRecPtr target_lsn, XLogRecPtr 
flushed_lsn,
 
        /*
         * Check if the standby slots have caught up to the flushed position. It
-        * is good to wait up to flushed position and then let it send the 
changes
-        * to logical subscribers one by one which are already covered in 
flushed
-        * position without needing to wait on every change for standby
-        * confirmation.
+        * is good to wait up to the flushed position and then let the WalSender
+        * send the changes to logical subscribers one by one which are already
+        * covered by the flushed position without needing to wait on every 
change
+        * for standby confirmation.
         */
        if (NeedToWaitForStandby(target_lsn, flushed_lsn, wait_event))
                return true;