v2-0001-Comments-improvement.patch.txt

From e03332839dd804f3bc38937e677ba87ac10f981b Mon Sep 17 00:00:00 2001
From: Shveta Malik <shveta.malik@gmail.com>
Date: Tue, 5 Mar 2024 11:29:52 +0530
Subject: [PATCH v2] Comments improvement.

---
 src/backend/replication/walsender.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 96f44ba85d..6a082b42eb 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1799,13 +1799,6 @@ NeedToWaitForWal(XLogRecPtr target_lsn, XLogRecPtr flushed_lsn,
 		return true;
 	}
 
-	/*
-	 * Check if the standby slots have caught up to the flushed position. It
-	 * 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 (NeedToWaitForStandbys(flushed_lsn, wait_event))
 		return true;
 
@@ -1818,7 +1811,10 @@ NeedToWaitForWal(XLogRecPtr target_lsn, XLogRecPtr flushed_lsn,
  *
  * If the walsender holds a logical slot that has enabled failover, we also
  * wait for all the specified streaming replication standby servers to
- * confirm receipt of WAL up to RecentFlushPtr.
+ * confirm receipt of WAL up to RecentFlushPtr. It's beneficial to wait
+ * here for the confirmation from standbys up to RecentFlushPtr rather
+ * than waiting in ReorderBufferCommit() before transmitting each
+ * change to logical subscribers, which is already covered in RecentFlushPtr.
  *
  * Returns end LSN of flushed WAL.  Normally this will be >= loc, but if we
  * detect a shutdown request (either from postmaster or client) we will return
@@ -1847,6 +1843,12 @@ WalSndWaitForWal(XLogRecPtr loc)
 	else
 		RecentFlushPtr = GetXLogReplayRecPtr(NULL);
 
+	/*
+	 * In the loop, we wait for the necessary WALs to be flushed to disk
+	 * initially, and then we wait for standbys to catch up. Upon receiving
+	 * the shutdown signal, we immediately transition to waiting for standbys
+	 * to catch up.
+	 */
 	for (;;)
 	{
 		bool		wait_for_standby_at_stop = false;
@@ -1877,12 +1879,10 @@ WalSndWaitForWal(XLogRecPtr loc)
 			XLogBackgroundFlush();
 
 		/*
-		 * Within the loop, we wait for the necessary WALs to be flushed to
-		 * disk first, followed by waiting for standbys to catch up if there
-		 * are enough WALs or upon receiving the shutdown signal. To avoid the
-		 * scenario where standbys need to catch up to a newer WAL location in
-		 * each iteration, we update our idea of the currently flushed
-		 * position only if we are not waiting for standbys to catch up.
+		 * Update our idea of the currently flushed position, but do it only
+		 * if we haven't started waiting for standbys. This is done to prevent
+		 * standbys from needing to catch up to a more recent WAL location
+		 * with each iteration.
 		 */
 		if (wait_event != WAIT_EVENT_WAIT_FOR_STANDBY_CONFIRMATION)
 		{
-- 
2.34.1