Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 15 Jan 2025 15:11:32 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: per backend WAL statistics
Message-ID: <Z4dRlNuhSQ3hPPv2@paquier.xyz>
References: <Z4BmdUQQh8jzlPHq@paquier.xyz>
 <Z4DrFhdlO6Xf13Xd@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="dEJPkOWmqRY5LZah"
Content-Disposition: inline
In-Reply-To: <Z4DrFhdlO6Xf13Xd@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz>
Precedence: bulk


--dEJPkOWmqRY5LZah
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jan 10, 2025 at 09:40:38AM +0000, Bertrand Drouvot wrote:
> Please find attached v4 taking into account 2c14037bb5.

+} PgStat_WalCounters;
+
+typedef struct PgStat_WalStats
+{
+	PgStat_WalCounters wal_counters;

I know that's a nit, but perhaps that could be a patch of its own,
pushing that to pg_stat_wal_build_tuple() to reduce the diffs in the
main patch.

- * Find or create a local PgStat_BackendPending entry for proc number.
+ * Find or create a local PgStat_BackendPendingIO entry for proc number.

Seems like you are undoing a change here.

+ * WAL pending statistics are incremented inside a critical section
+ * (see XLogWrite()), so we can't use pgstat_prep_pending_entry() and we rely on
+ * PendingBackendWalStats instead.
+ */
+extern PGDLLIMPORT PgStat_PendingWalStats PendingBackendWalStats;

Hmm.  This makes me wonder if we should rethink a bit the way pending
entries are retrieved and if we should do it beforehand for the WAL
paths to avoid allocations in some critical sections.  Isn't that also
because we avoid calling pgstat_prep_backend_pending() for the I/O
case as only backends are supported now, discarding cases like the
checkpointer where I/O could happen in a critical path?  As a whole,
the approach taken by the patch is not really consistent with the
rest.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmeHUZQACgkQnvQgOdby
QH2gkA/8CF1s6WV+jcgzeYYo6/fb6Mj731L7E69p1hh2n/VkucvyZwby7Q0xWj6O
yNEouaSPEVDXXzpiwLpb6TF+YlcvoZbgwjY3a/NSehjJv6J5xtUlHH7j5yqWV9Df
rRVTRWcU8q7l2zejbAjUte3ZS1gTHRCJMenHnNuGtJNs5BPP336gdiWb0XADBcVX
aW/HwYSePdKq919VMNDSjtDY3RcURoF+bB40qJeW1G0ze9I5OZVM6NvocajlJpq/
C0tCwlNxiXOmad4SiKIPPHVjXARO3EYsjHybX8SuOtKYM+2IDS77OA3QPUpfSG6t
LIsShmh/EclImc2OGD3zvoYOeImJxYDUbFYrkP/9rNab59woJfx00JoWbYlfIrON
3wMvPTGGTls22DtazXfufcVBvvwofonYshiWuahZ1is8914L9wNgNyQPQ+hMrKZB
O6IBqDGL7TvRCvXP5ZRZ4WewqGmxJBEprOYclA4li3TEqRknrkUXR3E3fD1TlraC
RNPaG/tTgMY5/GSxOyZaoiTLN6FY09baoD1B2cdEFkDeakmfwigSH+cVzQiNNuV+
ATCzV6l6ZlSdSA85zIjxAwdIa0EViGx53jEX7TuYSnH3n+BAQtJXEfilx1PBPTmJ
pKj6Vdv/vtR0+qdWsIZEcp0FnkPVDZKOQ6QDdJWNfyRb2c6LRck=
=W4v1
-----END PGP SIGNATURE-----

--dEJPkOWmqRY5LZah--