From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: Standby server with cascade logical replication could not be properly stopped under load
Date: Wed, 28 May 2025 19:12:25 +0300
Message-ID: <a4f477e0-5d17-466b-a8cc-e0d905d1161b@postgrespro.ru> (raw)
In-Reply-To: <aDa83q7wJt6II3Iq@paquier.xyz>
References: <52138028-7246-421c-9161-4fa108b88070@postgrespro.ru>
<CAJpy0uA9kw9WLWETPg+upwzBu145vMB7sPZwBuR_vDrbazeuag@mail.gmail.com>
<d9e1c765-7f83-4015-ade3-16506854d0b1@postgrespro.ru>
<aC9DXHt/MwFCW6Is@ip-10-97-1-34.eu-west-3.compute.internal>
<f2575904-acfe-4037-9748-61fdb4a7abe4@postgrespro.ru>
<aDa83q7wJt6II3Iq@paquier.xyz>
On 5/28/25 10:35, Michael Paquier wrote:
> Wouldn't it be better to extend GetStandbyFlushRecPtr() so as the
> caller can directly decide if they want to include in the maths the
> last LSN flushed by the WAL receiver, depending on the code path where
> the routine is called? That makes for a larger patch, of course, but
> it removes the need to look at MyWalSnd inside
> GetStandbyFlushRecPtr().
Well, this is a great question, because it made me thinking whether we
need to change behavior of THIS function at all. Extending
'GetStandbyFlushRecPtr' to properly support logical replication will
just make it equal to the 'GetXLogReplayRecPtr'. So, basically we just
need to replace invocation of 'GetStandbyFlushRecPtr' with
'GetXLogReplayRecPtr' and this should solve the problem.
The only invoker of 'GetStandbyFlushRecPtr' outside of walsender.c is
slot synchronization routine, which expects to get 'physical' position
anyway, so it does not need to be changed. All other invocations are
performed from the 'walsender.c' file itself. Obviously, both
'StartReplication' (physical) and 'XLogSendPhysical' functions need
current logic, so we can skip them. The 'XLogSendLogical' need to be
changed to use 'GetXLogReplayRecPtr' (to get consistent behavior with
WalSndWaitForWal and actually fix the original problem discussed in this
thread). This leaves us with only single function to consider:
'IdentifySystem'. This function could be called both in case of physical
and logical replication, but I assume that we need to return flushed WAL
position in it as well, as it's described in the documentation for this
command. This also seems to be the right behavior, as primary goal for
'identify' command is to inform client about the potential replication
start point and we definitely can start replication from the flushed
position (as we already has this record), just with some delay (once our
recovery get us to this point). But maybe somebody with more knowledge
of the logical replication expectation could confirm this.
The fix seems to be very simple in this case (assuming the logic above
is correct): we only replace 'GetStandbyFlushRecPtr' call in
'XLogSendLogical' with the 'GetXLogReplayRecPtr' and that's it. Attached
is the modified patch. It could be applied on top of 16, 17 and master
versions.
As for the test: do you want this case with shutdown to be added to the
existing test plan for standby logical recovery? The condition to
trigger the problem is just to have a flushed and replayed positions
pointing to different values during standby shutdown. I could try to add
it to the test suit, but it could take some time.
Thanks,
Alexey
Attachments:
[text/x-patch] 0001-Use-only-replayed-position-as-target-flush-point-4-pg-master.patch (2.3K, ../a4f477e0-5d17-466b-a8cc-e0d905d1161b@postgrespro.ru/2-0001-Use-only-replayed-position-as-target-flush-point-4-pg-master.patch)
download | inline diff:
From 1d91e067fad0bc68b55d6d771cc03078681de780 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Wed, 28 May 2025 17:45:52 +0300
Subject: [PATCH] Use only replayed position as target flush point for cascaded
logical replication
For physical walsender the GetStandbyFlushRecPtr function returns position
of the latest stored WAL record, as we can stream data to downstream instance
without waiting for it being locally applied. However, for cascaded logical
replication we can sent data to downstream client only up to the locally
applied position. This distinction is important when we decide whether
walsender has sent all the available data to client ('caught up'). During the
shutdown process walsender backends are allowed to work until they reach the
'caught up' state, while recovery process is already deactivated, so the
applied position won't be moving. In this case walsender for logical
replication should work only until it caught up with the latest applied record,
otherwise it will stuck in the infinite loop and inhibit instance shutdown.
To avoid such problems XLogSendLogical method should use GetXLogReplayRecPtr
function rather than GetStandbyFlushRecPtr.
---
src/backend/replication/walsender.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.cindex 9fa8beb6103..c7d475b99b6 100644--- a/src/backend/replication/walsender.c+++ b/src/backend/replication/walsender.c@@ -3449,8 +3449,17 @@ XLogSendLogical(void)
if (flushPtr == InvalidXLogRecPtr ||
logical_decoding_ctx->reader->EndRecPtr >= flushPtr)
{
+ /*+ * For cascaded logical replication we need to use replay position+ * rather than flushed one, as we could not decode and stream data+ * which has not yet been applied locally. This distinction is+ * especially imporant during shutdown sequence, as caught up state is+ * calculated based on flushPtr and using flush position in this+ * context will cause indefinite wait in walsender for records which+ * won't be applied due to shutdown.+ */
if (am_cascading_walsender)
- flushPtr = GetStandbyFlushRecPtr(NULL);+ flushPtr = GetXLogReplayRecPtr(NULL);
else
flushPtr = GetFlushRecPtr(NULL);
}
--
2.49.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: a.makhmutov@postgrespro.ru, michael@paquier.xyz, bertranddrouvot.pg@gmail.com, shveta.malik@gmail.com, itsajin@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: Standby server with cascade logical replication could not be properly stopped under load
In-Reply-To: <a4f477e0-5d17-466b-a8cc-e0d905d1161b@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox