Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 29 Oct 2025 14:01:20 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Noah Misch <noah@leadboat.com>
Cc: ryanzxg@gmail.com, Xuneng Zhou <xunengzhou@gmail.com>,
	pgsql-bugs@lists.postgresql.org, Euler Taveira <euler@eulerto.com>
Subject: Re: BUG #19093: Behavioral change in walreceiver termination between
 PostgreSQL 14.17 and 14.18
Message-ID: <aQGfoKGgmAbPATp5@paquier.xyz>
References: <19093-c4fff49a608f82a0@postgresql.org>
 <CABPTF7XKZV2d4eWhH0ybfv5J0hHyzPxPDmebzQXsSt-8XvO1sA@mail.gmail.com>
 <20251027041241.ab.nmisch@google.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="s6Wrd/rb3GTEk9FR"
Content-Disposition: inline
In-Reply-To: <20251027041241.ab.nmisch@google.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aQGfoKGgmAbPATp5%40paquier.xyz>
Precedence: bulk


--s6Wrd/rb3GTEk9FR
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Oct 26, 2025 at 09:12:41PM -0700, Noah Misch wrote:
> On Fri, Oct 24, 2025 at 02:20:39PM +0800, Xuneng Zhou wrote:
> (Long-term, in master only, perhaps we should introduce another status li=
ke
> 'connecting'.  Perhaps enact the connecting->streaming status transition =
just
> before tendering the first byte of streamed WAL to the startup process.
> Alternatively, enact that transition when the startup process accepts the
> first streamed byte.  Then your application's health check would get what=
 it
> wants.)

Having a "connecting" status here would make sense to me, yep.

> This change would be wrong if WALRCV_STOPPING were a reachable state here.
> That state is the startup process asking walreceiver to stop.  walreceive=
r may
> then still be installing segments, so this location would want to call
> XLogShutdownWalRcv() to wait for WALRCV_STOPPED.  That said, WALRCV_STOPP=
ING
> is a transient state while the startup process is in ShutdownWalRcv().  H=
ence,
> I expect STOPPING never appears here, and there's no bug.  An assertion m=
ay be
> in order.

A WAL receiver marked as in STOPPING state should never be reached in
this code path as far as I recall the matter.  An assertion sounds
like a cheap insurance anyway.

> Can you add a TAP test for this case?  Since it was wrong in v15+ for >3y=
 and
> wrong in v14 for 5mon before this report, clearly we had a blind spot.

Hmm.  I think that we could tweak the recovery 004_timeline_switch.pl
that has scenarios for TLI switches, which is what this is about.  And
there is no need to rely on a failover of a previous primary to a
promoted standby, we could do the same with a cascading standby.

Based on the logs I can see at 14.18:
2025-10-29 11:09:36.964 JST walreceiver[711] LOG:  replication
terminated by primary server
2025-10-29 11:09:36.964 JST walreceiver[711] DETAIL:  End of WAL
reached on timeline 1 at 0/03024D18.
2025-10-29 11:09:36.964 JST walreceiver[711] FATAL:  terminating
walreceiver process due to administrator command
2025-10-29 11:09:36.964 JST startup[710] LOG:  new target timeline is 2

And on older versions, at 14.17:
2025-10-29 11:14:32.745 JST walreceiver[5857] LOG:  replication
terminated by primary server
2025-10-29 11:14:32.745 JST walreceiver[5857] DETAIL:  End of WAL
reached on timeline 1 at 0/03024D18.
2025-10-29 11:14:32.745 JST startup[5856] LOG:  new target timeline
is 2=20

I was thinking about two options to provide some coverage:
- Check that the PID of the WAL receiver is still the same before and
after the TLI switch, with a restart or a reload after changing
primary_conninfo.  This cannot be made stable without an injection
point or equivalent to stop the WAL receiver from doing any job before=20
it does the TLI jump and trigger the code path we are discussing here.
- Check for the logs that we never issue a "terminating walreceiver
process due to administrator command".  This would be simpler in the
long term.

Hence I would suggest just the following addition:
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -66,6 +66,11 @@ my $result =3D
   $node_standby_2->safe_psql('postgres', "SELECT count(*) FROM tab_int");
 is($result, qq(2000), 'check content of standby 2');
=20
+# Check the logs, WAL receiver should not have been stopped.  There is no =
need
+# to rely on a position in the logs: a new log file is used on node restar=
t.
+ok( !$node_standby_2->log_contains(
+  "FATAL: .* terminating walreceiver process due to administrator command"=
),
+  'WAL receiver should not be stopped across timeline jumps');
=20
 # Ensure that a standby is able to follow a primary on a newer timeline
 # when WAL archiving is enabled.

Note: the issue is also reachable on REL_13_STABLE, through
69a498eb6465.  Are you confident that we would be able to get
something into this branch by the next minor release?  Based on the
state of the problem and the advanced analysis, it sounds to me that
we should be able to conclude by the middle of next week, or something
close to that.

> postgr.es/m/YyACvP++zgDphlcm@paquier.xyz discusses a
> "standby.signal+primary_conninfo" case.  How will this patch interact with
> that case?

This does not sound like an issue to me?  This mentions a case where
we don't have a restore_command and where changes are pushed to the
local pg_wal/ by an external source.  If a WAL receiver is waiting,
the patch means that ResetInstallXLogFileSegmentActive() would be
called and the InstallXLogFileSegmentActive flag reset.  I don't see
why that's not OK.  Or are you foreseeing something I don't?
--
Michael

--s6Wrd/rb3GTEk9FR
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmkBn6AACgkQnvQgOdby
QH2Bbg/8Dk3roHhWSEky6Yu9yawSw8C7y7T1Y5G3buWBK9dApmBtyLoBX4Dq/Ea1
20EJ6QfpGF3eZ7Vqt+rQ9ZYIIgkTBjlBmDzkbK9DXMirVDhUo6EBJZLj/NXBdvcE
Bj+eQdxyEze23y2heMIOOILakSs5ULSJadpzafXAEW6FeRMcxfYxYf1vYo6l7SrR
HfkW8jpvkqY/UUM+AwpmO439cuh8sx0wlR3HHlyeJMi+C3vtQNkMgsK7TSIm/W2S
9QAKH7UtUDOAtxj+E18Y8ylCdzObQWqRbekXixlmF1L5g1sSY7BIDa374v2nNRIp
l91x2jGsXh/mtIbD+FUH6tuB2+s7q1zDXh61IAAS9lj7PhZ/A4zVQWZ+0wumEbXt
bQCrL++D0Uh7S6W7J8rjufkjQI4KqdyO1BXAJXDfWG36xUJ3trmyd3Y/ze/2B/Dr
psFcAcUr/6QNEGoyBID0PM5t+mUwTaWs6H+TwnFeJhH+zaz26UZcp6u+vzkz5zSe
wehzE7m0VGM7oCpJI1yEnfYWzps6ix6ZelsUdVTHsQhH+h933Wp/qxzmoZc2OKnL
manAd8zPm1b822mLY2iVXAyCwY31cL1IodJyb4rEXuP0tkvxX5S2EBn9W6vkQVLn
Lbj+8L3805wh7TjMB8Eyp6OwA6pOaThwe+7A13R8NJTYnezN4oI=
=UVoQ
-----END PGP SIGNATURE-----

--s6Wrd/rb3GTEk9FR--