Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 26 Feb 2025 15:37:10 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG]: the walsender does not update its IO statistics until it
 exits
Message-ID: <Z762lusJXp_wNF_z@paquier.xyz>
References: <Z73IsKBceoVd4t55@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="qo3oKa0MXEQX4OaZ"
Content-Disposition: inline
In-Reply-To: <Z73IsKBceoVd4t55@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Z762lusJXp_wNF_z%40paquier.xyz>
Precedence: bulk


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

On Tue, Feb 25, 2025 at 01:42:08PM +0000, Bertrand Drouvot wrote:
> Now we can see that the numbers increased for the relation object and tha=
t we
> get non zeros numbers for the wal object too (which makes fully sense).
>=20
> With the attached patch applied, we would get the same numbers already in
> step 4. (means the stats are flushed without the need to wait for the wal=
sender
> to exit).

@@ -2793,6 +2794,12 @@ WalSndLoop(WalSndSendDataCallback send_data)
         if (pq_flush_if_writable() !=3D 0)
             WalSndShutdown();
=20
+        /*
+         * Report IO statistics
+         */
+        pgstat_flush_io(false);
+        (void) pgstat_flush_backend(false, PGSTAT_BACKEND_FLUSH_IO);
+
         /* If nothing remains to be sent right now ... */
         if (WalSndCaughtUp && !pq_is_send_pending())
         {

That's bad, worse for a logical WAL sender, because it means that we
have no idea what kind of I/O happens in this process until it exits,
and logical WAL senders could loop forever, since v16 where we've
begun tracking I/O.

A non-forced periodic flush like you are proposing here sounds OK to
me, but the position of the flush could be positioned better in the
loop.  If there is a SIGUSR2 (aka got_SIGUSR2 is true), WAL senders
would shut down, so it seems rather pointless to do a flush just
before exiting the process in WalSndDone(), no?  I'd suggest to move
the flush attempt closer to where we wait for some activity, just
after WalSndKeepaliveIfNecessary().
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAme+tpYACgkQnvQgOdby
QH0vPQ//fTykOI4AIJgDiyHx1dr87VIKrZ3spLIydQCxPAWG1HZ1RgqnLJZDVgk1
Mvf9ahL/aoXzYdndfhjebFHe8Mn9JLXgsdnQqcEvQgU4QmseO5nzAoPCFg2aBjab
tuKwVw97XsubjDna0fNAqjjGo+YMZWr9YNugV2J9k2u7Wl8lA9Byn1VwdAf36YPc
Sl8/3KSvdyr53GZs6ZUYl9uWYfK5VjOkKK+j/5NoISIPotxD+OJ90gO37y3nRrV9
yPk+DbUvkEOtkuVbBTUTHyCENB5+S015/B8Ck8dhjYY/WND+QkaKzWzFnY7PkO1X
C7mjSneZx2r2KmvWJbKaHtVD1vZxtlSRvNGNJI4ITNhDBUpvhVrY2EaC1B3vjrwg
kxqGCf02mMnALXcIQUr+WSaRJgtTOtrNIzzOQCsZailQRCHOEPsakqBrHYNTEkRB
Fo6cVfzU7l6Bh6K5LWlnu+XQhnCjWYKoYy4W4Z8qI3hcjSzzvJ69kSHfpIcHskhO
1x35ZxkyWIEG+kADAdZOjnPQfHex1qeO+rchzAMScD2Q/1FGbrCZOXM0rKY1Yrwy
5g32DOTTc4Zv23oiP7z40hvAb00D0UWIOWxyUB9uAK2JhNodmC8wqu4IrHBEzrXA
2i5rdZHmTPuGccDEifoN66KosdeABo2P/UrYb0ScZQqXHSkBVxI=
=ec25
-----END PGP SIGNATURE-----

--qo3oKa0MXEQX4OaZ--