Feedback-ID: i0fe9450f:Fastmail
Date: Sat, 18 Jan 2025 17:53:31 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
	Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: per backend I/O statistics
Message-ID: <Z4tsCxa0dG6FdA4h@paquier.xyz>
References: <Z2KDy56Ls4nBl1Y8@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z2OfYh3QJ7M5D0i-@paquier.xyz>
 <CAN55FZ0ZQTf-D+5Rup4H6zp+n3E2EuonYuzTiPkp=1GhesZU_A@mail.gmail.com>
 <Z4dyHHxGfGK7bGjd@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z4d_eggsxtBEdJAG@paquier.xyz>
 <66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5>
 <Z4mfYkpzm5Fe44lZ@paquier.xyz>
 <Z4nza3pgLpD+rmrE@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z4n01COjAA4WYC6e@paquier.xyz>
 <Z4ovtCrpJCikdMM3@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="f9Kk0M4NVoBI1YOo"
Content-Disposition: inline
In-Reply-To: <Z4ovtCrpJCikdMM3@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Z4tsCxa0dG6FdA4h%40paquier.xyz>
Precedence: bulk


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

On Fri, Jan 17, 2025 at 10:23:48AM +0000, Bertrand Drouvot wrote:
> Please find attached a patch implementing the ideas above, meaning:
>=20
> - It creates a new PendingBackendStats variable
> - It uses this variable to increment and flush per backend pending IO sta=
tistics
>=20
> That way we get rid of the memory allocation for pending IO statistics.
>=20
> One remark: a special case has been added in pgstat_flush_pending_entries=
(). The
> reason is that while the per backend stats are "variable-numbered" stats,=
 it could
> be that their pending stats are not part of pgStatPending. This is curren=
tly the
> case (with this patch) as the per backend pending IO stats are not tracke=
d with
> pgstat_prep_backend_pending() and friends anymore.


+    /*
+     * There is a special case for some pending stats that are tracked in
+     * PendingBackendStats. It's possible that those have not been flushed
+     * above, hence the extra check here.
+     */
+    if (!pg_memory_is_all_zeros(&PendingBackendStats,
+                                sizeof(struct PgStat_BackendPending)))
+    {
+        PgStat_EntryRef *entry_ref;
+
+        entry_ref =3D pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
+                                         MyProcNumber, false, NULL);

Hmm.  Such special complexities in pgstat.c are annoying.  There is
a stupid thing I am wondering here.  For the WAL stats, why couldn't
we place some calls of pgstat_prep_backend_pending() in strategic
places like XLogBeginInsert() to force all the allocation steps of the
pending entry to happen before we would enter the critical sections
when doing a WAL insertion?  As far as I can see, there is a special
case with 2PC where XLogBeginInsert() could be called in a critical
section, but that seems to be the only one at quick glance.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmeLbAsACgkQnvQgOdby
QH33xQ/+JBW4fQDHmpLJTqLz0FAMm2erOk05Ege2rcNyziRJ/UngnlBzuwMgDsao
jrGH6L8412PfVuU9fVvfR+OR8n1hYtsi0KlfoHDn+K+TnYhTn33Z31YFC1sP3+Gc
9gBXNVen88tIs8BSsc+6WDKe8Xj3wtZjs53tVUWImchw3vZ1YEnd4kFj43z67/Ri
0f1EjNXbufEHJmCBkEiuKMpncp/nzpeQX2cfMgwFUEbxVw1giY+0KX90nYPenIcr
6R3EsZwiaantSMVnrRXCZwZ3r50PU25FL4L5BXGyoqQKPcCHmz+jY+hGvVhtvtpS
paVeEcSB04i/azZBHoMXyTVcN38HpT2Z2ud6h1HP8/k7Hxg0cFepgEI/BLjar+6D
5yvaPZxyDPxvD8DwKaOpyU6UbjhAaDMsPGdLJbhs4t+FCT99jNeBZ7FkyMYHuAgV
A1uoxWv2cJ4+YmRM5klilv56oSmqMnVVwvSvQFjWbOD0yvw6w4lS6+Ns5qS0RC/L
7F9bcdFtz+aZkPddf9I6fvcuIh8VXd70gzSiY8a2cWmrut8CSbwZfhO7pQxRDBEX
VwZp9gr3gLvvHvO4aNN1Ho0CuKXvAjm3qizfiz15wBHExkForQWPr7tY9SAtg7/B
Z6DWkAIHYL7osjmT41Q4RS87TJxuMlf1hg7ENeqPNzo87LnxOps=
=Htpw
-----END PGP SIGNATURE-----

--f9Kk0M4NVoBI1YOo--