Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 15 Jan 2025 18:27:22 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-hackers@lists.postgresql.org,
	Andres Freund <andres@anarazel.de>
Subject: Re: per backend I/O statistics
Message-ID: <Z4d_eggsxtBEdJAG@paquier.xyz>
References: <Z2BKTK9KX5Kc3HaU@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z2EZegYbH22LQHhN@paquier.xyz>
 <Z2EyxyEJPMMldj1t@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z2FAyFSg-lRGFZio@paquier.xyz>
 <Z2FF6ScWef9yIrJL@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z2JWUaY8AHBs99yp@paquier.xyz>
 <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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="cQ4TuUGeJ7KeJmoM"
Content-Disposition: inline
In-Reply-To: <Z4dyHHxGfGK7bGjd@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Z4d_eggsxtBEdJAG%40paquier.xyz>
Precedence: bulk


--cQ4TuUGeJ7KeJmoM
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 15, 2025 at 08:30:20AM +0000, Bertrand Drouvot wrote:
> On Wed, Jan 15, 2025 at 11:03:54AM +0300, Nazir Bilal Yavuz wrote:
>> With this commit it may not be possible to count IOs in the critical
>> sections. I think the problem happens only if the local
>> PgStat_BackendPending entry is being created for the first time for
>> this backend in the critical section.
>=20
> Yeah, I encountered the exact same thing and mentioned it in [1] (see R1.=
).
>=20
> In [1]=A0I did propose to use a new PendingBackendWalStats variable to "b=
ypass" the
> pgstat_prep_backend_pending() usage.
>=20
> Michael mentioned in [2] that is not really consistent with the rest (wha=
t I
> agree with) and that "we should rethink a bit the way pending entries are
> retrieved". I did not think about it yet but that might be the way to
> go, thoughts?=20
>=20
> [1]: https://www.postgresql.org/message-id/Z3zqc4o09dM/Ezyz%40ip-10-97-1-=
34.eu-west-3.compute.internal
> [2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz

My problem is that this is not only related to backend stats, but to
all variable-numbered stats kinds that require this behavior.  One
other case where I've seen this as being an issue is injection point
stats, for example, while enabling these stats for all callbacks and
some of them are run in critical sections.

A generic solution to the problem would be to allow
pgStatPendingContext to have MemoryContextAllowInCriticalSection() set
temporarily for the allocation of the pending data.
Perhaps not for all stats kinds, just for these where we're OK with
the potential memory footprint so we could have a flag in
PgStat_KindInfo.  Giving to all stats kinds the responsibility to
allocate a pending entry beforehand outside a critical section is
another option, but that means going through different tweaks for all
stats kinds that require them.

Andres, do you have any thoughts about that?  Not sure what would be=20
your view on this matter.  MemoryContextAllowInCriticalSection() for
stats kinds that allow the case would be OK for me I guess, we should
not be talking about a lot of memory.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmeHf3oACgkQnvQgOdby
QH21nxAAlUmZXC9rjQqt5uTiIIECXs7mu2IP4lyU1doDoOnvy+3JtedfufssXNAO
KFhKUW99hH8NJb10sxCLLgZ4iJSZKLX+Iy2CDWBsatGydXBgbrq2trlF6Q0KSgfj
JcjykbqhukmAhv5iwqsXLJI07d/5H+snHmbxmvta9iLyra0roIOguJZzIFAmONib
KRW03HtPUkXG6PMxW0VitOnNHMdHwrDj5lebWl9kWoUqzVY43iiKP/Vomo4RHMFu
alk6/jKBqC6w7X0DR57hjM0n8liRWJl3pDh6e0d7aH6ZveU7RfNIsEVeMoVU9ige
aclOupji6w/QMSFQy/wzK2LaCHxSsdi5OjKRMlseqyByU2oVvLN8DV3EdwC34frX
Vfb9DjBnN4uu2UguX1V3Izau6hv4zTUmjDrWG4DwRCa+fdPsVegazRTay9kJpgzj
LX+hRLsNaT5sDsFM6qwantp1t3MLEvKak25mcNqwaleuv1rcDYcC9N0oHgBrfAdM
PmV/BUTQEIfPtpbaVOlTNSVWJAkoTmCauBpwqRsvj2BXify0e+75jXsZ1uuVKvja
laAaZYpg+jKI93c5hUhiIn22XNkdP5Rj8xPfvjV1xTve95Flb+xH4sD+ayhFSwgk
GJPv+AVY/7F1DCLO8Iqvnx61Qy83iaQyBoxRr2P+I2ny8XqUjkc=
=mMxp
-----END PGP SIGNATURE-----

--cQ4TuUGeJ7KeJmoM--