pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Standby server with cascade logical replication could not be properly stopped under load
16+ messages / 6 participants
[nested] [flat]

* Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 02:19  Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  0 siblings, 2 replies; 16+ messages in thread

From: Alexey Makhmutov @ 2025-05-22 02:19 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org

Assuming following configuration with three connected servers A->B->C: A 
(primary), B (physical standby) and C (logical replica connected to B). 
If server A is under load and B is applying incoming WAL records while 
also streaming data via logical replication to C, then attempt to stop 
server B in 'fast' mode may by unsuccessful. In this case server will 
remain in PM_SHUTDOWN state indefinitely with all 'walsender' processes 
running in an infinite busy loop (consuming a CPU core each). To get 
server out of this state it's required either to either stop B using 
'immediate' mode or stop server C (which will cause 'walsender' 
processes on server B to exit). This issue is reproducible on latest 
'master', as well as on current PG 16/17 branches.

Attached is a test scenario to reproduce the issue: 'test_scenario.zip'. 
This archive contains shell scripts to create
the required environment (all three serves) and then to execute required 
steps to get server into incorrect state. First, edit the 'test_env.sh' 
file and specify path to PG binaries in PG_PATH variable and optionally 
set of ports used by test instances in 'pg_port' array. Then execute the 
'test_prepare.sh' script, which will create, configure and start all 
three PG instances. Servers then could be started and stopped using 
corresponding start and stop scripts. To reproduce the issue, ensure 
that all three servers are running and execute the 'test_execute.sh' 
script. This script starts 'pgbench' instance in background for 30 
seconds to create load on server A, waits for 20 seconds and then try to 
stop the B instance using default 'fast' mode. Expected behavior is 
normal shutdown for B, while observed behavior is different: shutdown 
attempt fails and each remaining 'walsender' process consumes entire CPU 
core. To get out of this state one could use 'stop-C.sh' script to stop 
the server C, as it will complete shutdown process of B instance as well.

My understanding is that this issue seems to be caused by the logic in 
'GetStandbyFlushRecPtr' function, which returns current flush point for 
received WAL data. This position is used in 'XLogSendLogical' to 
calculate whether current walsender is in 'caught up' state (i.e. 
whether we send all the available data to downstream instance). During 
shutdown walsenders are allowed to continue their work until they are in 
'caught up' state, while 'postmaster' is waiting for their completion. 
Currently 'GetStandbyFlushRecPtr' returns position of last stored 
record, rather than last applied record. This is correct for physical 
replication as we can send data to downstream instance without applying 
it to local system. However, for logical replication this seems to be 
incorrect, as we could not decode data until it's applied on current 
instance. So, if current stored WAL position differs from applied 
position while server is being stopped, then 
'WalSndLoop'/'XLogSendLogical'/'XLogReadRecord' methods will spin in a 
busy loop, waiting for applied position to advance. The recovery process 
is already stopped at this moment, so this will be an infinite loop. 
Probably either 'GetStandbyFlushRecPtr' or 
'WalSndLoop'/'XLogSendLogical' logic need to be adjusted to take into 
consideration such case with  logical replication.

Attached is also a patch, which aims to fix this issue: 
0001-Use-only-replayed-position-as-target-flush-point-for.patch. It 
tries to to modify behavior of 'GetStandbyFlushRecPtr' function to 
ensure that it returns only applied position for logical replication. 
This function could be also invoked from slot synchronization routines 
and in this case it retains current behavior by returning last stored 
position.

Thanks,
Alexey

Attachments:

  [application/zip] test_scenario.zip (5.1K, ../../52138028-7246-421c-9161-4fa108b88070@postgrespro.ru/2-test_scenario.zip)
  download

  [text/x-patch] 0001-Use-only-replayed-position-as-target-flush-point-for.patch (2.9K, ../../52138028-7246-421c-9161-4fa108b88070@postgrespro.ru/3-0001-Use-only-replayed-position-as-target-flush-point-for.patch)
  download | inline diff:
From 698b13005e6fb85741b758d9f8b14065f6394092 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Thu, 22 May 2025 04:02:50 +0300
Subject: [PATCH] Use only replayed position as target flush point for 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 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.
---
 src/backend/replication/walsender.c | 31 ++++++++++++++++-------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9fa8beb6103..ede985da216 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3532,29 +3532,32 @@ WalSndDone(WalSndSendDataCallback send_data)
 XLogRecPtr
 GetStandbyFlushRecPtr(TimeLineID *tli)
 {
-	XLogRecPtr	replayPtr;
 	TimeLineID	replayTLI;
-	XLogRecPtr	receivePtr;
-	TimeLineID	receiveTLI;
 	XLogRecPtr	result;
 
 	Assert(am_cascading_walsender || IsSyncingReplicationSlots());
 
-	/*
-	 * We can safely send what's already been replayed. Also, if walreceiver
-	 * is streaming WAL from the same timeline, we can send anything that it
-	 * has streamed, but hasn't been replayed yet.
-	 */
-
-	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
-	replayPtr = GetXLogReplayRecPtr(&replayTLI);
+	result = GetXLogReplayRecPtr(&replayTLI);
 
 	if (tli)
 		*tli = replayTLI;
 
-	result = replayPtr;
-	if (receiveTLI == replayTLI && receivePtr > replayPtr)
-		result = receivePtr;
+	/*
+	 * We can safely send what's already been replayed. Also, for physical
+	 * replication if walreceiver is streaming WAL from the same timeline, we
+	 * can send anything that it has streamed, but hasn't been replayed yet.
+	 * If this function is called from slot synchronization (MyWalSnd is NULL
+	 * in this case), then we should follow physical replication approach as
+	 * well to be consistent with synchronized_standby_slots expectations.
+	 */
+	if (!MyWalSnd || MyWalSnd->kind == REPLICATION_KIND_PHYSICAL)
+	{
+		TimeLineID	receiveTLI;
+		XLogRecPtr	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
+
+		if (receiveTLI == replayTLI && receivePtr > result)
+			result = receivePtr;
+	}
 
 	return result;
 }
-- 
2.49.0



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 09:48  Ajin Cherian <itsajin@gmail.com>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  1 sibling, 0 replies; 16+ messages in thread

From: Ajin Cherian @ 2025-05-22 09:48 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: pgsql-bugs@lists.postgresql.org

On Thu, May 22, 2025 at 7:25 PM Alexey Makhmutov
<a.makhmutov@postgrespro.ru> wrote:
>
> Assuming following configuration with three connected servers A->B->C: A
> (primary), B (physical standby) and C (logical replica connected to B).
> If server A is under load and B is applying incoming WAL records while
> also streaming data via logical replication to C, then attempt to stop
> server B in 'fast' mode may by unsuccessful. In this case server will
> remain in PM_SHUTDOWN state indefinitely with all 'walsender' processes
> running in an infinite busy loop (consuming a CPU core each). To get
> server out of this state it's required either to either stop B using
> 'immediate' mode or stop server C (which will cause 'walsender'
> processes on server B to exit). This issue is reproducible on latest
> 'master', as well as on current PG 16/17 branches.
>
> Attached is a test scenario to reproduce the issue: 'test_scenario.zip'.
> This archive contains shell scripts to create
> the required environment (all three serves) and then to execute required
> steps to get server into incorrect state. First, edit the 'test_env.sh'
> file and specify path to PG binaries in PG_PATH variable and optionally
> set of ports used by test instances in 'pg_port' array. Then execute the
> 'test_prepare.sh' script, which will create, configure and start all
> three PG instances. Servers then could be started and stopped using
> corresponding start and stop scripts. To reproduce the issue, ensure
> that all three servers are running and execute the 'test_execute.sh'
> script. This script starts 'pgbench' instance in background for 30
> seconds to create load on server A, waits for 20 seconds and then try to
> stop the B instance using default 'fast' mode. Expected behavior is
> normal shutdown for B, while observed behavior is different: shutdown
> attempt fails and each remaining 'walsender' process consumes entire CPU
> core. To get out of this state one could use 'stop-C.sh' script to stop
> the server C, as it will complete shutdown process of B instance as well.
>
> My understanding is that this issue seems to be caused by the logic in
> 'GetStandbyFlushRecPtr' function, which returns current flush point for
> received WAL data. This position is used in 'XLogSendLogical' to
> calculate whether current walsender is in 'caught up' state (i.e.
> whether we send all the available data to downstream instance). During
> shutdown walsenders are allowed to continue their work until they are in
> 'caught up' state, while 'postmaster' is waiting for their completion.
> Currently 'GetStandbyFlushRecPtr' returns position of last stored
> record, rather than last applied record. This is correct for physical
> replication as we can send data to downstream instance without applying
> it to local system. However, for logical replication this seems to be
> incorrect, as we could not decode data until it's applied on current
> instance. So, if current stored WAL position differs from applied
> position while server is being stopped, then
> 'WalSndLoop'/'XLogSendLogical'/'XLogReadRecord' methods will spin in a
> busy loop, waiting for applied position to advance. The recovery process
> is already stopped at this moment, so this will be an infinite loop.
> Probably either 'GetStandbyFlushRecPtr' or
> 'WalSndLoop'/'XLogSendLogical' logic need to be adjusted to take into
> consideration such case with  logical replication.
>
> Attached is also a patch, which aims to fix this issue:
> 0001-Use-only-replayed-position-as-target-flush-point-for.patch. It
> tries to to modify behavior of 'GetStandbyFlushRecPtr' function to
> ensure that it returns only applied position for logical replication.
> This function could be also invoked from slot synchronization routines
> and in this case it retains current behavior by returning last stored
> position.

Good catch, I agree with the analysis. I've also verified that the fix
works as expected.
Just a small comment: can you explicitly mention in the comments that
"for logical replication we can only send records that have already
been replayed else we might get stuck in shutdown" or something to
that effect as that distinction is important for future developers in
this area.

regards,
Ajin Cherian
Fujitsu Australia





^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 10:33  shveta malik <shveta.malik@gmail.com>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  1 sibling, 1 reply; 16+ messages in thread

From: shveta malik @ 2025-05-22 10:33 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org, shveta malik <shveta.malik@gmail.com>

On Thu, May 22, 2025 at 7:51 AM Alexey Makhmutov
<a.makhmutov@postgrespro.ru> wrote:
>
> Assuming following configuration with three connected servers A->B->C: A
> (primary), B (physical standby) and C (logical replica connected to B).
> If server A is under load and B is applying incoming WAL records while
> also streaming data via logical replication to C, then attempt to stop
> server B in 'fast' mode may by unsuccessful. In this case server will
> remain in PM_SHUTDOWN state indefinitely with all 'walsender' processes
> running in an infinite busy loop (consuming a CPU core each). To get
> server out of this state it's required either to either stop B using
> 'immediate' mode or stop server C (which will cause 'walsender'
> processes on server B to exit). This issue is reproducible on latest
> 'master', as well as on current PG 16/17 branches.
>
> Attached is a test scenario to reproduce the issue: 'test_scenario.zip'.
> This archive contains shell scripts to create
> the required environment (all three serves) and then to execute required
> steps to get server into incorrect state. First, edit the 'test_env.sh'
> file and specify path to PG binaries in PG_PATH variable and optionally
> set of ports used by test instances in 'pg_port' array. Then execute the
> 'test_prepare.sh' script, which will create, configure and start all
> three PG instances. Servers then could be started and stopped using
> corresponding start and stop scripts. To reproduce the issue, ensure
> that all three servers are running and execute the 'test_execute.sh'
> script. This script starts 'pgbench' instance in background for 30
> seconds to create load on server A, waits for 20 seconds and then try to
> stop the B instance using default 'fast' mode. Expected behavior is
> normal shutdown for B, while observed behavior is different: shutdown
> attempt fails and each remaining 'walsender' process consumes entire CPU
> core. To get out of this state one could use 'stop-C.sh' script to stop
> the server C, as it will complete shutdown process of B instance as well.
>
> My understanding is that this issue seems to be caused by the logic in
> 'GetStandbyFlushRecPtr' function, which returns current flush point for
> received WAL data. This position is used in 'XLogSendLogical' to
> calculate whether current walsender is in 'caught up' state (i.e.
> whether we send all the available data to downstream instance). During
> shutdown walsenders are allowed to continue their work until they are in
> 'caught up' state, while 'postmaster' is waiting for their completion.
> Currently 'GetStandbyFlushRecPtr' returns position of last stored
> record, rather than last applied record. This is correct for physical
> replication as we can send data to downstream instance without applying
> it to local system. However, for logical replication this seems to be
> incorrect, as we could not decode data until it's applied on current
> instance. So, if current stored WAL position differs from applied
> position while server is being stopped, then
> 'WalSndLoop'/'XLogSendLogical'/'XLogReadRecord' methods will spin in a
> busy loop, waiting for applied position to advance. The recovery process
> is already stopped at this moment, so this will be an infinite loop.
> Probably either 'GetStandbyFlushRecPtr' or
> 'WalSndLoop'/'XLogSendLogical' logic need to be adjusted to take into
> consideration such case with  logical replication.
>
> Attached is also a patch, which aims to fix this issue:
> 0001-Use-only-replayed-position-as-target-flush-point-for.patch. It
> tries to to modify behavior of 'GetStandbyFlushRecPtr' function to
> ensure that it returns only applied position for logical replication.
> This function could be also invoked from slot synchronization routines
> and in this case it retains current behavior by returning last stored
> position.
>

The problem stated in 'logical-walsender' on 'physical standby' looks
genuine. I agree with the analysis for slot-sync as well. Slot-sync
does not need the fix as it deals only with flush-position and does
not care about replay-position. Since the problem area falls under
'Allow logical decoding on standbys', I am adding Bertrand for further
comments on this fix.

thanks
Shveta





^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 13:29  Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  parent: shveta malik <shveta.malik@gmail.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Alexey Makhmutov @ 2025-05-22 13:29 UTC (permalink / raw)
  To: shveta malik <shveta.malik@gmail.com>; Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>; Ajin Cherian <itsajin@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org

Hi Ajin and Shveta, thank you for looking at this issue.

On 5/22/25 12:48, Ajin Cherian wrote:

> Just a small comment: can you explicitly mention in the comments that
> "for logical replication we can only send records that have already
> been replayed else we might get stuck in shutdown" or something to
> that effect as that distinction is important for future developers in
> this area.

Thank you. I've expanded the comment to make it more verbose to clarify 
expected logic in this case.
Updated patch is attached as 
'0001-Use-only-replayed-position-as-target-flush-point-2.patch'.

Thanks,
Alexey

Attachments:

  [text/x-patch] 0001-Use-only-replayed-position-as-target-flush-point-2.patch (3.2K, ../../d9e1c765-7f83-4015-ade3-16506854d0b1@postgrespro.ru/2-0001-Use-only-replayed-position-as-target-flush-point-2.patch)
  download | inline diff:
From 12f98aadb5a423b17b41e65d36c80355be1beb61 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Thu, 22 May 2025 04:02:50 +0300
Subject: [PATCH] Use only replayed position as target flush point for 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 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.
---
 src/backend/replication/walsender.c | 36 ++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9fa8beb6103..3c480d02393 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3532,29 +3532,37 @@ WalSndDone(WalSndSendDataCallback send_data)
 XLogRecPtr
 GetStandbyFlushRecPtr(TimeLineID *tli)
 {
-	XLogRecPtr	replayPtr;
 	TimeLineID	replayTLI;
-	XLogRecPtr	receivePtr;
-	TimeLineID	receiveTLI;
 	XLogRecPtr	result;
 
 	Assert(am_cascading_walsender || IsSyncingReplicationSlots());
 
-	/*
-	 * We can safely send what's already been replayed. Also, if walreceiver
-	 * is streaming WAL from the same timeline, we can send anything that it
-	 * has streamed, but hasn't been replayed yet.
-	 */
-
-	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
-	replayPtr = GetXLogReplayRecPtr(&replayTLI);
+	result = GetXLogReplayRecPtr(&replayTLI);
 
 	if (tli)
 		*tli = replayTLI;
 
-	result = replayPtr;
-	if (receiveTLI == replayTLI && receivePtr > replayPtr)
-		result = receivePtr;
+	/*
+	 * For physical replication we can send downstream anything that has
+	 * already been received if walreceiver is streaming WAL from the same
+	 * timeline. This is also true if this function is called from slot
+	 * synchronization routines (MyWalSnd is NULL in such case), since they
+	 * also only track the physically flushed position. However, for logical
+	 * replication the situation is different and we can only decode and send
+	 * downstream data that has already been applied to the current instance.
+	 * This distinction is especially important during shutdown, as walsender
+	 * is allowed to continue its work until data is sent to the client up to
+	 * the flush point, so it can get stuck if we return stored rather than
+	 * replayed data position.
+	 */
+	if (!MyWalSnd || MyWalSnd->kind == REPLICATION_KIND_PHYSICAL)
+	{
+		TimeLineID	receiveTLI;
+		XLogRecPtr	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
+
+		if (receiveTLI == replayTLI && receivePtr > result)
+			result = receivePtr;
+	}
 
 	return result;
 }
-- 
2.49.0



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 15:31  Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  0 siblings, 1 reply; 16+ messages in thread

From: Bertrand Drouvot @ 2025-05-22 15:31 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi,

On Thu, May 22, 2025 at 04:29:37PM +0300, Alexey Makhmutov wrote:
> Hi Ajin and Shveta, thank you for looking at this issue.
> 
> On 5/22/25 12:48, Ajin Cherian wrote:
> 
> > Just a small comment: can you explicitly mention in the comments that
> > "for logical replication we can only send records that have already
> > been replayed else we might get stuck in shutdown" or something to
> > that effect as that distinction is important for future developers in
> > this area.
> 
> Thank you. I've expanded the comment to make it more verbose to clarify
> expected logic in this case.
> Updated patch is attached as
> '0001-Use-only-replayed-position-as-target-flush-point-2.patch'.

Thanks for the report!

I agree with your analysis and I think that your proposed fix make sense. I
wonder if the comment on top of GetStandbyFlushRecPtr() could be updated a bit
though.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com





^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 18:22  Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  parent: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
  0 siblings, 2 replies; 16+ messages in thread

From: Alexey Makhmutov @ 2025-05-22 18:22 UTC (permalink / raw)
  To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi Bertrand, thank you for looking at this issue.

On 5/22/25 18:31, Bertrand Drouvot wrote:
> I agree with your analysis and I think that your proposed fix make sense. I
> wonder if the comment on top of GetStandbyFlushRecPtr() could be updated a bit
> though.

I've tried to update method comment as well in the updated patch: 
0001-Use-only-replayed-position-as-target-flush-point-3-pg-master.patch.
The same patch could be applied on top of PG 17 as well.

PG 16 does not have the slot synchronization logic, so its 
implementation of GetStandbyFlushRecPtr is slightly different and 
comments need to be updated to reflect this discrepancy as well. So, 
here is a variant of the same patch updated to be compatible with PG 16: 
0002-Use-only-replayed-position-as-target-flush-point-3-pg-16.patch

Thanks,
Alexey

Attachments:

  [text/x-patch] 0001-Use-only-replayed-position-as-target-flush-point-3-pg-master.patch (4.0K, ../../f2575904-acfe-4037-9748-61fdb4a7abe4@postgrespro.ru/2-0001-Use-only-replayed-position-as-target-flush-point-3-pg-master.patch)
  download | inline diff:
From 52fe2c5094c3a65ff8bafced4b0f522007068ebb Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Thu, 22 May 2025 04:02:50 +0300
Subject: [PATCH] Use only replayed position as target flush point for 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 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.
---
 src/backend/replication/walsender.c | 49 ++++++++++++++++-------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9fa8beb6103..49d5b3fea5a 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3519,42 +3519,49 @@ WalSndDone(WalSndSendDataCallback send_data)
 }
 
 /*
- * Returns the latest point in WAL that has been safely flushed to disk.
- * This should only be called when in recovery.
- *
  * This is called either by cascading walsender to find WAL position to be sent
- * to a cascaded standby or by slot synchronization operation to validate remote
- * slot's lsn before syncing it locally.
+ * to a downstream server or by slot synchronization operation to validate remote
+ * slot's lsn before syncing it locally. For streaming replication it returns
+ * latest point in WAL that has been safely flushed to disk and for logical
+ * replication it returns latest replayed position. This should only be called
+ * when in recovery.
  *
- * As a side-effect, *tli is updated to the TLI of the last
- * replayed WAL record.
+ * As a side-effect, *tli is updated to the TLI of the last replayed WAL record.
  */
 XLogRecPtr
 GetStandbyFlushRecPtr(TimeLineID *tli)
 {
-	XLogRecPtr	replayPtr;
 	TimeLineID	replayTLI;
-	XLogRecPtr	receivePtr;
-	TimeLineID	receiveTLI;
 	XLogRecPtr	result;
 
 	Assert(am_cascading_walsender || IsSyncingReplicationSlots());
 
-	/*
-	 * We can safely send what's already been replayed. Also, if walreceiver
-	 * is streaming WAL from the same timeline, we can send anything that it
-	 * has streamed, but hasn't been replayed yet.
-	 */
-
-	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
-	replayPtr = GetXLogReplayRecPtr(&replayTLI);
+	result = GetXLogReplayRecPtr(&replayTLI);
 
 	if (tli)
 		*tli = replayTLI;
 
-	result = replayPtr;
-	if (receiveTLI == replayTLI && receivePtr > replayPtr)
-		result = receivePtr;
+	/*
+	 * For physical replication we can send downstream anything that has
+	 * already been received if walreceiver is streaming WAL from the same
+	 * timeline. This is also true if this function is called from slot
+	 * synchronization routines (MyWalSnd is NULL in such case), since they
+	 * also only track the physically flushed position. However, for logical
+	 * replication the situation is different and we can only decode and send
+	 * downstream data that has already been applied to the current instance.
+	 * This distinction is especially important during shutdown, as walsender
+	 * is allowed to continue its work until data is sent to the client up to
+	 * the flush point, so it can get stuck if we return stored rather than
+	 * replayed data position.
+	 */
+	if (!MyWalSnd || MyWalSnd->kind == REPLICATION_KIND_PHYSICAL)
+	{
+		TimeLineID	receiveTLI;
+		XLogRecPtr	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
+
+		if (receiveTLI == replayTLI && receivePtr > result)
+			result = receivePtr;
+	}
 
 	return result;
 }
-- 
2.49.0



  [text/x-patch] 0002-Use-only-replayed-position-as-target-flush-point-3-pg-16.patch (3.6K, ../../f2575904-acfe-4037-9748-61fdb4a7abe4@postgrespro.ru/3-0002-Use-only-replayed-position-as-target-flush-point-3-pg-16.patch)
  download | inline diff:
From ad8085d7646401f62d9b17af5274041bff77a720 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Thu, 22 May 2025 04:02:50 +0300
Subject: [PATCH] Use only replayed position as target flush point for 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 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.
---
 src/backend/replication/walsender.c | 43 ++++++++++++++++-------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 1e0480b1d55..f91ba63454c 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3189,37 +3189,42 @@ WalSndDone(WalSndSendDataCallback send_data)
 }
 
 /*
- * Returns the latest point in WAL that has been safely flushed to disk, and
- * can be sent to the standby. This should only be called when in recovery,
- * ie. we're streaming to a cascaded standby.
+ * This is called by cascading walsender to find WAL position to be sent to a
+ * downstream server. For streaming replication it returns* latest point in WAL
+ * that has been safely flushed to disk and for logical replication it returns
+ * latest replayed position. This should only be called when in recovery.
  *
- * As a side-effect, *tli is updated to the TLI of the last
- * replayed WAL record.
+ * As a side-effect, *tli is updated to the TLI of the last replayed WAL record.
  */
 static XLogRecPtr
 GetStandbyFlushRecPtr(TimeLineID *tli)
 {
-	XLogRecPtr	replayPtr;
 	TimeLineID	replayTLI;
-	XLogRecPtr	receivePtr;
-	TimeLineID	receiveTLI;
 	XLogRecPtr	result;
 
-	/*
-	 * We can safely send what's already been replayed. Also, if walreceiver
-	 * is streaming WAL from the same timeline, we can send anything that it
-	 * has streamed, but hasn't been replayed yet.
-	 */
-
-	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
-	replayPtr = GetXLogReplayRecPtr(&replayTLI);
+	result = GetXLogReplayRecPtr(&replayTLI);
 
 	if (tli)
 		*tli = replayTLI;
 
-	result = replayPtr;
-	if (receiveTLI == replayTLI && receivePtr > replayPtr)
-		result = receivePtr;
+	/*
+	 * For physical replication we can send downstream anything that has
+	 * already been received if walreceiver is streaming WAL from the same
+	 * timeline. However, for logical replication the situation is different
+	 * and we can only decode and send downstream data that has already been
+	 * applied to the current instance. This distinction is especially
+	 * important during shutdown, as walsender is allowed to continue its work
+	 * until data is sent to the client up to the flush point, so it can get
+	 * stuck if we return stored rather than replayed data position.
+	 */
+	if (MyWalSnd->kind == REPLICATION_KIND_PHYSICAL)
+	{
+		TimeLineID	receiveTLI;
+		XLogRecPtr	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
+
+		if (receiveTLI == replayTLI && receivePtr > result)
+			result = receivePtr;
+	}
 
 	return result;
 }
-- 
2.49.0



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-22 23:47  Michael Paquier <michael@paquier.xyz>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  1 sibling, 0 replies; 16+ messages in thread

From: Michael Paquier @ 2025-05-22 23:47 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

On Thu, May 22, 2025 at 09:22:02PM +0300, Alexey Makhmutov wrote:
> I've tried to update method comment as well in the updated patch:
> 0001-Use-only-replayed-position-as-target-flush-point-3-pg-master.patch.
> The same patch could be applied on top of PG 17 as well.
> 
> PG 16 does not have the slot synchronization logic, so its implementation of
> GetStandbyFlushRecPtr is slightly different and comments need to be updated
> to reflect this discrepancy as well. So, here is a variant of the same patch
> updated to be compatible with PG 16:
> 0002-Use-only-replayed-position-as-target-flush-point-3-pg-16.patch

Hmm.  The scenario you are pointing at with a mix of cascading logical
and physical standbys is tricky enough that I think this warrants
having a test in the long-run.  Do you think that it would be possible
to sort something out with ordered actions taken during the shutdown
sequences at least for HEAD?

Are v15 and older versions things that we should worry about when
using setups made of physical standbys?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aC-3nJekGZaAfNjQ@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-28 07:35  Michael Paquier <michael@paquier.xyz>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  1 sibling, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2025-05-28 07:35 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

On Thu, May 22, 2025 at 09:22:02PM +0300, Alexey Makhmutov wrote:
> I've tried to update method comment as well in the updated patch:
> 0001-Use-only-replayed-position-as-target-flush-point-3-pg-master.patch.
> The same patch could be applied on top of PG 17 as well.

+	if (MyWalSnd->kind == REPLICATION_KIND_PHYSICAL)
+	{
+		TimeLineID	receiveTLI;
+		XLogRecPtr	receivePtr = GetWalRcvFlushRecPtr(NULL, &receiveTLI);
+
+		if (receiveTLI == replayTLI && receivePtr > result)
+			result = receivePtr;
+	}

Looking at the patch, I am not much a fan of looking at MyWalSnd in
GetStandbyFlushRecPtr() which does not have such a dependency yet, to
then decide if the LSN retrieved by GetWalRcvFlushRecPtr() should be
removed or used.

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().
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aDa83q7wJt6II3Iq@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-28 16:12  Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 16+ messages in thread

From: Alexey Makhmutov @ 2025-05-28 16:12 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

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.c
index 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



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-29 07:01  Michael Paquier <michael@paquier.xyz>
  parent: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2025-05-29 07:01 UTC (permalink / raw)
  To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; Ajin Cherian <itsajin@gmail.com>; pgsql-bugs@lists.postgresql.org

On Wed, May 28, 2025 at 07:12:25PM +0300, Alexey Makhmutov wrote:
> 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.

Exactly.  I don't want to assume that all the callers are OK with this
forced changed in the context of a logical WAL sender inside the
function GetStandbyFlushRecPtr(), especially since the change can be
isolated where it only makes sense, aka inside the logical sender
loop.

> 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.

I don't think that there is any need to care about IDENTIFY_SYSTEM.
It would be a short-lived command.  Note that we document that the LSN
returned should be the current flush LSN, that could be used as a
starting point for START_REPLICATION.  START_REPLICATION has different
properties and assumptions; it's expected to last with its inner logic
required for the shutdown sequence order with WAL senders.

> 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.

Yeah, I think that this is a sensible move.  Documenting the reason
why GetXLogReplayRecPtr() is called is important, but the comment you
are suggesting could be better, IMO, say:
"For cascading logical WAL senders, we use the replay LSN rather than
the flush LSN, as decoding would only happen with records already
replayed.  This distinction is important especially during the
shutdown sequence, as cascading logical WAL senders can only catch up
with records that have been already replayed, not flushed."

That feels that I'm repeating myself a bit twice if formulated this
way.  If you have a better suggestion, feel free..

> 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.

Anything I could think of for the moment would require the release of
an injection point, which is not something we can do during a shutdown
sequence with the existing facility as we cannot connect to the server
while it's in this late in its shutdown sequence: new connections are
prohibited once we expect the WAL senders to catch up.  We could go
without that for now to get a fix in, your scripts are enough to
push all the logical WAL senders on the physical standby in an
infinite loop once its shutdown sequence initiates.

Bertrand and others, what do you think?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aDgGXArm-CGh1pSG@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-29 10:28  Ajin Cherian <itsajin@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 16+ messages in thread

From: Ajin Cherian @ 2025-05-29 10:28 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

On Thu, May 29, 2025 at 5:02 PM Michael Paquier <michael@paquier.xyz> wrote:

> nd master versions.
>
> Yeah, I think that this is a sensible move.  Documenting the reason
> why GetXLogReplayRecPtr() is called is important, but the comment you
> are suggesting could be better, IMO, say:
> "For cascading logical WAL senders, we use the replay LSN rather than
> the flush LSN, as decoding would only happen with records already
> replayed.  This distinction is important especially during the
> shutdown sequence, as cascading logical WAL senders can only catch up
> with records that have been already replayed, not flushed."
>
> That feels that I'm repeating myself a bit twice if formulated this
> way.  If you have a better suggestion, feel free..
>
>
> I think this new fix is much better and cleaner. A suggestion for the
comment:
"For cascading logical WAL senders, we use the replay LSN instead of the
flush LSN, since logical decoding on a standby only processes WAL that has
been replayed. This distinction becomes particularly important during
shutdown, as new WAL is no longer replayed and the last replayed LSN marks
the furthest point up to which decoding can proceed."

regards,
Ajin Cherian
Fujitsu Australia

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-05-29 23:32  Michael Paquier <michael@paquier.xyz>
  parent: Ajin Cherian <itsajin@gmail.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2025-05-29 23:32 UTC (permalink / raw)
  To: Ajin Cherian <itsajin@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

On Thu, May 29, 2025 at 08:28:01PM +1000, Ajin Cherian wrote:
> I think this new fix is much better and cleaner. A suggestion for the
> comment:
> "For cascading logical WAL senders, we use the replay LSN instead of the
> flush LSN, since logical decoding on a standby only processes WAL that has
> been replayed. This distinction becomes particularly important during
> shutdown, as new WAL is no longer replayed and the last replayed LSN marks
> the furthest point up to which decoding can proceed."

Yes, that sounds better than my previous suggestion.  Thanks.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aDjudRqTXYJCt3VT@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-06-02 03:17  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2025-06-02 03:17 UTC (permalink / raw)
  To: Ajin Cherian <itsajin@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

On Fri, May 30, 2025 at 08:32:05AM +0900, Michael Paquier wrote:
> On Thu, May 29, 2025 at 08:28:01PM +1000, Ajin Cherian wrote:
>> I think this new fix is much better and cleaner. A suggestion for the
>> comment:
>> "For cascading logical WAL senders, we use the replay LSN instead of the
>> flush LSN, since logical decoding on a standby only processes WAL that has
>> been replayed. This distinction becomes particularly important during
>> shutdown, as new WAL is no longer replayed and the last replayed LSN marks
>> the furthest point up to which decoding can proceed."
> 
> Yes, that sounds better than my previous suggestion.  Thanks.

I have reused that wording, did more tests on v16 and v17, reproducing
both the problems reported and that the change fixes the shutdown of
the physical standby, then applied the change down to v16.

Thanks all.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aD0Xvh4Tj_Vk7pCA@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-12-26 11:14  G. Sl <skokoveshat@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 16+ messages in thread

From: G. Sl @ 2025-12-26 11:14 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Ajin Cherian <itsajin@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi Michael !
I've found the same behaviour in the 15.12 version. Any chances for
backpatch for this version ?
-- 
Best regards
Gennadiy S.
On Fri, 26 Dec 2025 at 16:11, Michael Paquier <michael@paquier.xyz> wrote:

> On Fri, May 30, 2025 at 08:32:05AM +0900, Michael Paquier wrote:
> > On Thu, May 29, 2025 at 08:28:01PM +1000, Ajin Cherian wrote:
> >> I think this new fix is much better and cleaner. A suggestion for the
> >> comment:
> >> "For cascading logical WAL senders, we use the replay LSN instead of the
> >> flush LSN, since logical decoding on a standby only processes WAL that
> has
> >> been replayed. This distinction becomes particularly important during
> >> shutdown, as new WAL is no longer replayed and the last replayed LSN
> marks
> >> the furthest point up to which decoding can proceed."
> >
> > Yes, that sounds better than my previous suggestion.  Thanks.
>
> I have reused that wording, did more tests on v16 and v17, reproducing
> both the problems reported and that the change fixes the shutdown of
> the physical standby, then applied the change down to v16.
>
> Thanks all.
> --
> Michael
>

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-12-29 00:09  Michael Paquier <michael@paquier.xyz>
  parent: G. Sl <skokoveshat@gmail.com>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2025-12-29 00:09 UTC (permalink / raw)
  To: G. Sl <skokoveshat@gmail.com>; +Cc: Ajin Cherian <itsajin@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

On Fri, Dec 26, 2025 at 04:14:17PM +0500, G. Sl wrote:
> I've found the same behaviour in the 15.12 version. Any chances for
> backpatch for this version ?

As presented on this thread, the problem reported involves at least a
cluster configuration A -> B -> C, where:
- A is a primary.
- B is a physical standby, streaming changes from A.  In the test
setup, this node includes replication slots, that have been created
while the node is in standby mode.  This action can only happen in v16
and newer versions, where logical decoding on standbys has been added.
- C is a primary node, streaming logical changes from B.

Based on how the state of B required, this would not apply to v15
because it is not possible to create replication slots on a standby.
Also you may want to update to 15.15 first.

I am open to arguments or discussions if you have found a new or
different problem, of course, but there is nothing we can do without
knowing more about your setup.  An even better thing would be to have 
a reproducible test case based on v15 to understand your problem, but
I can say for sure that we may be dealing with something different.
Please feel free to refer to the top of the thread, which offers an
excellent example of what a reproducible test case can be.  By that I
mean something that we can reuse and reproduce the issue.

Also note that the change committed is 5231ed8262c9, that relies on
the existence of am_cascading_walsender in XLogSendLogical() to not
use GetFlushRecPtr() in all cases.  Before the commit, we used
GetStandbyFlushRecPtr() for that in v16.  Again, v15 just uses
GetFlushRecPtr(), but it should not be possible to find yourself in a
position where XLogSendLogical() is called while on a standby, no?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../aVHGz-twKB-1EHE-@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Standby server with cascade logical replication could not be properly stopped under load
@ 2025-12-29 05:22  G. Sl <skokoveshat@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 16+ messages in thread

From: G. Sl @ 2025-12-29 05:22 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Ajin Cherian <itsajin@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; shveta malik <shveta.malik@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi Michael ! Thx for clarifying.
No, I don't have a replica with logical slots, just a primary with logical
replication ( cdc debezium ) and sometimes it shutdown(restart) hangs until
killing of replication processes.
So it looks like I should investigate further and maybe, create a new issue.

On Mon, 29 Dec 2025 at 05:10, Michael Paquier <michael@paquier.xyz> wrote:

> On Fri, Dec 26, 2025 at 04:14:17PM +0500, G. Sl wrote:
> > I've found the same behaviour in the 15.12 version. Any chances for
> > backpatch for this version ?
>
> As presented on this thread, the problem reported involves at least a
> cluster configuration A -> B -> C, where:
> - A is a primary.
> - B is a physical standby, streaming changes from A.  In the test
> setup, this node includes replication slots, that have been created
> while the node is in standby mode.  This action can only happen in v16
> and newer versions, where logical decoding on standbys has been added.
> - C is a primary node, streaming logical changes from B.
>
> Based on how the state of B required, this would not apply to v15
> because it is not possible to create replication slots on a standby.
> Also you may want to update to 15.15 first.
>
> I am open to arguments or discussions if you have found a new or
> different problem, of course, but there is nothing we can do without
> knowing more about your setup.  An even better thing would be to have
> a reproducible test case based on v15 to understand your problem, but
> I can say for sure that we may be dealing with something different.
> Please feel free to refer to the top of the thread, which offers an
> excellent example of what a reproducible test case can be.  By that I
> mean something that we can reuse and reproduce the issue.
>
> Also note that the change committed is 5231ed8262c9, that relies on
> the existence of am_cascading_walsender in XLogSendLogical() to not
> use GetFlushRecPtr() in all cases.  Before the commit, we used
> GetStandbyFlushRecPtr() for that in v16.  Again, v15 just uses
> GetFlushRecPtr(), but it should not be possible to find yourself in a
> position where XLogSendLogical() is called while on a standby, no?
> --
> Michael
>

^ permalink  raw  reply  [nested|flat] 16+ messages in thread


end of thread, other threads:[~2025-12-29 05:22 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-22 02:19 Standby server with cascade logical replication could not be properly stopped under load Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2025-05-22 09:48 ` Ajin Cherian <itsajin@gmail.com>
2025-05-22 10:33 ` shveta malik <shveta.malik@gmail.com>
2025-05-22 13:29   ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2025-05-22 15:31     ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2025-05-22 18:22       ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2025-05-22 23:47         ` Michael Paquier <michael@paquier.xyz>
2025-05-28 07:35         ` Michael Paquier <michael@paquier.xyz>
2025-05-28 16:12           ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2025-05-29 07:01             ` Michael Paquier <michael@paquier.xyz>
2025-05-29 10:28               ` Ajin Cherian <itsajin@gmail.com>
2025-05-29 23:32                 ` Michael Paquier <michael@paquier.xyz>
2025-06-02 03:17                   ` Michael Paquier <michael@paquier.xyz>
2025-12-26 11:14                     ` G. Sl <skokoveshat@gmail.com>
2025-12-29 00:09                       ` Michael Paquier <michael@paquier.xyz>
2025-12-29 05:22                         ` G. Sl <skokoveshat@gmail.com>

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