public inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Xuneng Zhou <xunengzhou@gmail.com>
Subject: Re: Fix pg_stat_wal_receiver to show CONNECTING status
Date: Thu, 21 May 2026 21:08:54 +0900
Message-ID: <ag711gjcxuaBqpp8@paquier.xyz> (raw)
In-Reply-To: <E2A0E5FB-E841-4D4B-96F8-B4016359CDF0@gmail.com>
References: <EF91FF76-1E2B-4F3B-9162-290B4DC517FF@gmail.com>
	<agxr29Hsz7FjxzlN@paquier.xyz>
	<1F153E64-B791-42FA-A60A-64813B20B81E@gmail.com>
	<ag00OeH1sbt5ie_6@paquier.xyz>
	<75CDE990-29D5-4D5C-BFE1-3840F19C0163@gmail.com>
	<ag4dCGAPBc5VFhCi@paquier.xyz>
	<1B695040-F544-447C-A6A8-C8BFF7F799D1@gmail.com>
	<E2A0E5FB-E841-4D4B-96F8-B4016359CDF0@gmail.com>

On Thu, May 21, 2026 at 03:20:13PM +0800, Chao Li wrote:
> I spent more time here, and found that it is still possible to leak
> conninfo in the WAL receiver reuse path:
> 
> * WalRcvWaitForStartPosition() sets the state to WALRCV_WAITING.
> * Then RequestXLogStreaming() copies raw conninfo into
> * walrcv->conninfo and sets the state to WALRCV_RESTARTING.
> * WalRcvWaitForStartPosition() then moves the state to
> * WALRCV_CONNECTING, but this path does not clear walrcv->conninfo
> * again.
> 
> The attached nocfbot_test.diff demonstrates the leak.

File is missing, but I get it.  This is a legit bug from what I can
see, that also affects all the stable branches, not only HEAD.

> Initially I thought we could also set ready_to_display to false when
> setting the state to WALRCV_WAITING in WalRcvWaitForStartPosition(),
> and set it back to true when switching back to
> WALRCV_CONNECTING. However, that would make the WALRCV_WAITING and
> WALRCV_RESTARTING states invisible in pg_stat_wal_receiver.

Nah, we should not do that.  We want to track the waiting and
restarting states in the view.

> I ended up with a solution that copies the primary connection info
> to walrcv->conninfo only when RequestXLogStreaming() is switching to
> WALRCV_STARTING. In the WALRCV_WAITING reuse path, the WAL receiver
> keeps using the existing wrconn, so it does not need raw conninfo to
> be copied into shared memory again. See the attached
> nocfbot_walreceiverfuncs.c.diff.

Ah, yeah.  This solution to this problem makes sense.  We should not
clobber conninfo either in this case, or we'd lose the
user-displayable string returned by walrcv_get_conninfo() (conninfo
cannot be NULL based on the in-core callers of RequestXLogStreaming()
AFAIK, but who knows for things out there).  As mentioned above, this
is a different issue than the visibility of the connection information
while we are connecting, and it should be backpatched.  Would you like
to send a patch?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

reply

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-hackers@postgresql.org
  Cc: michael@paquier.xyz, li.evan.chao@gmail.com, michael.paquier@gmail.com, xunengzhou@gmail.com
  Subject: Re: Fix pg_stat_wal_receiver to show CONNECTING status
  In-Reply-To: <ag711gjcxuaBqpp8@paquier.xyz>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox