Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 17 Jan 2025 09:08:02 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
	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: <Z4mfYkpzm5Fe44lZ@paquier.xyz>
References: <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>
 <Z4d_eggsxtBEdJAG@paquier.xyz>
 <66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="R8ElHb1mQYDiqmny"
Content-Disposition: inline
In-Reply-To: <66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5>
Archived-At: <https://www.postgresql.org/message-id/Z4mfYkpzm5Fe44lZ%40paquier.xyz>
Precedence: bulk


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

On Thu, Jan 16, 2025 at 11:28:43AM -0500, Andres Freund wrote:
> On 2025-01-15 18:27:22 +0900, Michael Paquier wrote:
>> My problem is that this is not only related to backend stats, but to
>> all variable-numbered stats kinds that require this behavior.
>=20
> The issue here imo is that recently IO stats were turned into a variable
> numbered stat, where it previously wasn't. It seems like a bad idea that =
now
> we regularly do a somewhat sizable memory allocation for backend stats. F=
or
> table stats allocating the pending memory is somewhat a necessity - there
> could be a lot of different tables after all. And later transactions won't
> necessarily access the same set of tables, so we can't just keep the memo=
ry
> for pending around - but that's not true for IO.

Yep, for the reason that backend stats require that with one entry in
the pgstats dshash for each backend as these are sized depending on
max_connections & co using a proc number for the hash key.

>> Andres, do you have any thoughts about that?  Not sure what would be
>> 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.
>=20
> My view is that for IO stats no memory allocation should be required - th=
at
> used to be the case and should be the case again. And for something like
> injection points (not really convinced that it's a good thing to have sta=
ts
> for, but ...), allocation the shared memory stats object outside of the
> critical section and not having any pending stats seems to be the right
> approach.

Not sure to completely agree on that as this pushes the responsibility
of how to deal with the critical section handling to each stats kind,
but I won't fight over it for this case, either.  It would not be the
first time we have specific logic to do allocations beforehand when
dealing with critical sections (38c579b08988 is the most recent
example coming in mind, there are many others).=20

Anyway, let's just switch pgstat_backend.c so as we use a static
PgStat_BackendPending (pending name) to store the pending IO stats
that could be reused for also the WAL bits.  It does not seem that
complicated as far as I can see, removing pgstat_prep_backend_pending
and changing pgstat_flush_backend_entry_io to use the static pending
data.  The patch set posted here is doing that within its
pgstat_flush_backend_entry_wal():
https://www.postgresql.org/message-id/Z4DrFhdlO6Xf13Xd@ip-10-97-1-34.eu-wes=
t-3.compute.internal

I could tweak that around the beginning of next week with a proposal
of patch.  Bertrand, perhaps you'd prefer hack on this one?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmeJn2IACgkQnvQgOdby
QH1XwA/6A3Ug3dRTcNDzpy+kxPMao6KuuFtlSIpUPTRVf8XSFKk2wBtinUWNzXQ+
xN8E754JzuhDD8eH0Ql4k9yULmsZzbxVEOCGLNYaibT8SGPvp8sxvEDD5wRyDBeG
Y7HPpCwoZAXJAgncgwEs7Dbv7Q3HknU1i0jyADVxsKeVeg+TcUEYhNCDuIIAeSWj
afEgGeZUCOj1gr4ELAQJXT9NUD2sZcP1kDUOsUtJVBXpXT/B6CUY/og4kNZrJlrW
2lROnfoGppnSIOsTDAz+ML/4W18NKhKN3a3zVr9nBoAJKUdQWIX3q0W8whDZOb30
lbgBCUtixGRqbtwZUnfc7+RjfWAXfko5SE6hoFU1tY/PmC+PyMjFGTmFF1cmUpew
c19XMiuO1zWeaqLKsci8i62Ob+TQ4zKAzm20SrNF6OW0vsGC3kHRIr7ksuBaREeR
8iobLOuLySPEKXcegYOIErklWYiuWU7IeG/WjQgjnNaFUPWeld0NyhAN0g+mCGeJ
Wq0yF7ENZUk6GCwQrsZngA+Btz0wH9fax6SwCps32vCGzGwSaXoh3CXJk95qhjeC
dOgXCk2ydVvPNxRp9I2Y22/oQt+V6I5AGTXYAtD9Qr87u1GprucVUAKfTkFSYJAt
VaLmAc8y0vSn7zskc9JYrRxm7+z7uqRb9Ya2B20UJ0QivMY0Mt0=
=DmSn
-----END PGP SIGNATURE-----

--R8ElHb1mQYDiqmny--