From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 6 Mar 2014 19:13:38 +0000 (+0200)
Subject: Send keepalives from walsender even when busy sending WAL.
X-Git-Tag: REL9_4_BETA1~374
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=94ae6ba74dfc626efa271461902db1be35d2a551;p=postgresql

Send keepalives from walsender even when busy sending WAL.

If walsender doesn't hear from the client for the time specified by
wal_sender_timeout, it will conclude the connection or client is dead, and
disconnect. When half of wal_sender_timeout has elapsed, it sends a ping
to the client, leaving it the remainig half of wal_sender_timeout to
respond. However, it only checked if half of wal_sender_timeout had elapsed
when it was about to sleep, so if it was busy sending WAL to the client for
long enough, it would not send the ping request in time. Then the client
would not know it needs to send a reply, and the walsender will disconnect
even though the client is still alive. Fix that.

Andres Freund, reviewed by Robert Haas, and some further changes by me.
Backpatch to 9.3. Earlier versions relied on the client to send the
keepalives on its own, and hence didn't have this problem.
---

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 4fcf3d4376..003c797e0e 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1258,6 +1258,27 @@ WalSndLoop(void)
 			}
 		}
 
+		/*
+		 * If half of wal_sender_timeout has elapsed without receiving any
+		 * reply from standby, send a keep-alive message requesting an
+		 * immediate reply.
+		 */
+		if (wal_sender_timeout > 0 && !ping_sent)
+		{
+			TimestampTz timeout;
+
+			timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
+												  wal_sender_timeout / 2);
+			if (GetCurrentTimestamp() >= timeout)
+			{
+				WalSndKeepalive(true);
+				ping_sent = true;
+				/* Try to flush pending output to the client */
+				if (pq_flush_if_writable() != 0)
+					goto send_failure;
+			}
+		}
+
 		/*
 		 * We don't block if not caught up, unless there is unsent data
 		 * pending in which case we'd better block until the socket is
@@ -1267,7 +1288,7 @@ WalSndLoop(void)
 		 */
 		if ((caughtup && !streamingDoneSending) || pq_is_send_pending())
 		{
-			TimestampTz timeout = 0;
+			TimestampTz timeout;
 			long		sleeptime = 10000;		/* 10 s */
 			int			wakeEvents;
 
@@ -1276,32 +1297,14 @@ WalSndLoop(void)
 
 			if (pq_is_send_pending())
 				wakeEvents |= WL_SOCKET_WRITEABLE;
-			else if (wal_sender_timeout > 0 && !ping_sent)
-			{
-				/*
-				 * If half of wal_sender_timeout has lapsed without receiving
-				 * any reply from standby, send a keep-alive message to
-				 * standby requesting an immediate reply.
-				 */
-				timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
-													  wal_sender_timeout / 2);
-				if (GetCurrentTimestamp() >= timeout)
-				{
-					WalSndKeepalive(true);
-					ping_sent = true;
-					/* Try to flush pending output to the client */
-					if (pq_flush_if_writable() != 0)
-						goto send_failure;
-				}
-			}
 
-			/* Determine time until replication timeout */
+			/*
+			 * If wal_sender_timeout is active, sleep in smaller increments
+			 * to not go over the timeout too much. XXX: Why not just sleep
+			 * until the timeout has elapsed?
+			 */
 			if (wal_sender_timeout > 0)
-			{
-				timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
-													  wal_sender_timeout);
 				sleeptime = 1 + (wal_sender_timeout / 10);
-			}
 
 			/* Sleep until something happens or we time out */
 			ImmediateInterruptOK = true;
@@ -1315,6 +1318,8 @@ WalSndLoop(void)
 			 * possibility that the client replied just as we reached the
 			 * timeout ... he's supposed to reply *before* that.
 			 */
+			timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
+												  wal_sender_timeout);
 			if (wal_sender_timeout > 0 && GetCurrentTimestamp() >= timeout)
 			{
 				/*