database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-05 09:19:17 +03:00
Code

walsnd: Don't set waiting_for_ping_response spuriously

Browse Source 
Ashutosh Bapat noticed that when logical walsender needs to wait for
WAL, and it realizes that it must send a keepalive message to
walreceiver to update the sent-LSN, which *does not* request a reply
from walreceiver, it wrongly sets the flag that it's going to wait for
that reply.  That means that any future would-be sender of feedback
messages ends up not sending a feedback message, because they all
believe that a reply is expected.

With built-in logical replication there's not much harm in this, because
WalReceiverMain will send a ping-back every wal_receiver_timeout/2
anyway; but with other logical replication systems (e.g. pglogical) it
can cause significant pain.

This problem was introduced in commit 41d5f8ad734, where the
request-reply flag was changed from true to false to WalSndKeepalive,
without at the same time removing the line that sets
waiting_for_ping_response.

Just removing that line would be a sufficient fix, but it seems better
to shift the responsibility of setting the flag to WalSndKeepalive
itself instead of requiring caller to do it; this is clearly less
error-prone.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
Backpatch: 9.5 and up
Discussion: https://postgr.es/m/20200806225558.GA22401@alvherre.pgsql
This commit is contained in:
Alvaro Herrera 2020-08-08 12:31:55 -04:00
parent f1c3a41bd6
commit 900429d0c0
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
1 changed files with 12 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/backend/replication/walsender.c
View File

@ -151,7 +151,7 @@ static XLogRecPtr sendTimeLineValidUpto = InvalidXLogRecPtr;
* How far have we sent WAL already? This is also advertised in * How far have we sent WAL already? This is also advertised in
* MyWalSnd->sentPtr. (Actually, this is the next WAL location to send.) * MyWalSnd->sentPtr. (Actually, this is the next WAL location to send.)
*/ */
static XLogRecPtr sentPtr = 0; static XLogRecPtr sentPtr = InvalidXLogRecPtr;
/* Buffers for constructing outgoing messages and processing reply messages. */ /* Buffers for constructing outgoing messages and processing reply messages. */
static StringInfoData output_message; static StringInfoData output_message;
@ -1451,10 +1451,7 @@ WalSndWaitForWal(XLogRecPtr loc)
if (MyWalSnd->flush < sentPtr && if (MyWalSnd->flush < sentPtr &&
MyWalSnd->write < sentPtr && MyWalSnd->write < sentPtr &&
!waiting_for_ping_response) !waiting_for_ping_response)
{
WalSndKeepalive(false); WalSndKeepalive(false);
waiting_for_ping_response = true;
}
/* check whether we're done */ /* check whether we're done */
if (loc <= RecentFlushPtr) if (loc <= RecentFlushPtr)
@ -2932,10 +2929,7 @@ WalSndDone(WalSndSendDataCallback send_data)
proc_exit(0); proc_exit(0);
} }
if (!waiting_for_ping_response) if (!waiting_for_ping_response)
{
WalSndKeepalive(true); WalSndKeepalive(true);
waiting_for_ping_response = true;
}
} }
/* /*
@ -3432,10 +3426,13 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
} }
/* /*
* This function is used to send a keepalive message to standby. * Send a keepalive message to standby.
* If requestReply is set, sets a flag in the message requesting the standby *
* to send a message back to us, for heartbeat purposes. * If requestReply is set, the message requests the other party to send
*/ * a message back to us, for heartbeat purposes. We also set a flag to
* let nearby code that we're waiting for that response, to avoid
* repeated requests.
*/
static void static void
WalSndKeepalive(bool requestReply) WalSndKeepalive(bool requestReply)
{ {
@ -3450,6 +3447,10 @@ WalSndKeepalive(bool requestReply)
/* ... and send it wrapped in CopyData */ /* ... and send it wrapped in CopyData */
pq_putmessage_noblock('d', output_message.data, output_message.len); pq_putmessage_noblock('d', output_message.data, output_message.len);
/* Set local flag */
if (requestReply)
waiting_for_ping_response = true;
} }
/* /*
@ -3480,7 +3481,6 @@ WalSndKeepaliveIfNecessary(void)
if (last_processing >= ping_time) if (last_processing >= ping_time)
{ {
WalSndKeepalive(true); WalSndKeepalive(true);
waiting_for_ping_response = true;
/* Try to flush pending output to the client */ /* Try to flush pending output to the client */
if (pq_flush_if_writable() != 0) if (pq_flush_if_writable() != 0)