Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 16 Dec 2024 17:07:52 +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
Subject: Re: per backend I/O statistics
Message-ID: <Z1_f2FzvUoL9KZsQ@paquier.xyz>
References: <Zz_gHQ2g8MUe8oJ1@paquier.xyz>
 <Z0A3pmORCbhRe0Ug@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z0PNpAjhcS7S-1LI@paquier.xyz>
 <Z0QjeIkwC0HNI16K@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z0Qk3nZcaVpbtCKU@paquier.xyz>
 <Z0ScL5W6EeWXShb3@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z1pr86DCR_P6iTSV@paquier.xyz>
 <Z1rs/j5JMdTbUIJJ@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z1uVzXFE9WhSicOx@paquier.xyz>
 <Z1v8TQbeTyHH5kxU@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="2JGdTg12ZKrKP4HO"
Content-Disposition: inline
In-Reply-To: <Z1v8TQbeTyHH5kxU@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Z1_f2FzvUoL9KZsQ%40paquier.xyz>
Precedence: bulk


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

On Fri, Dec 13, 2024 at 09:20:13AM +0000, Bertrand Drouvot wrote:
> Yeah makes sense, that's consistent with other kinds: done.

This looks to be taking shape.  I don't have much more comments.

Not feeling so sure about the value brought by the backend_type
returned in pg_stat_get_backend_io(), but well..

+	/* drop the backend stats entry */
+	pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber);

Oh, I've missed something.  Shouldn't pgstat_request_entry_refs_gc()
be called when this returns false?

The creation of the dshash entry is a bit too early, I think..  How
about delaying it more so as we don't create entries that could be
useless if we fail the last steps of authentication?  One spot would
be to delay the creation of the new entry at the end of
pgstat_bestart(), where we know that we are done with authentication
and that the backend is ready to report back to the client connected
to it.  It is true that some subsystems could produce stats as of the=20
early transactions they generate, which is why pgstat_initialize() is
done that early in BaseInit(), but that's not really relevant for this
case?

I'm still feeling a bit uneasy about the drop done in
pgstat_shutdown_hook(); it would be nice to make sure that this
happens in a path that would run just after we're done with the
creation of the entry to limit windows where we have an entry but no
way to drop it, or vice-versa, so as the shutdown assertions would
never trigger.  Perhaps there's an argument for an entirely separate
callback that would run before pgstat is plugged off, like a new
before_shmem_exit() callback registered after the entry is created?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmdf39gACgkQnvQgOdby
QH0oERAAoJoxg6o6o7I6NKOPHxLxKFhaczcfiBRzw1SSdF20k9QTcK26yf4skpQQ
KiSg5K4/kZHTWTBgGoAH/lnRqG82vuNM3VeIM8/5naKEqIjaNx1GMKot74AWiRaZ
xBbfozNoKhkowRQij+VhBYFqI4eDs7U2E2iZlcoC7RvHSr/PktVgYXSMJcPyjb4X
tps+AyA6u4g6B/D/JYMNOsBFvUC6AHbeUEgz7HW2vEvIgWoU4is32Rdn9RnVeAnV
NUtDjIuH9NiRjUTZTuRBZFhAV6bU+jjkzuy/OwAjZBcpzDTC1obTkLncxf/0N/p/
oi8ip8EgAAbmUwacw/FIoMI+SnTG1s+hRfWV3BHhm4zuU8MPWHwTkVHtJsRjKbAB
+JAc2perULqfI1NiQ49i9P8HHRGvmYMClxDFAbwSNfgAqBCK/zTw57oYbOONA30h
Ziy1BbKBtD7hEw9Cq16kPaun8RlLCm5bVKvHIcH59G710DfvCadbM6i7IZ1xCJUO
d+XQhntACokvgtZPfh1fvDwu2IrEFL2WxwbjHd8bY3GBhhwSVT4Xm6Y2xxSiV8+1
EzWcgQ/Ppws4XRLPlHVqW2fG83idXkgdMYugtXZhp3WnXT4GEypD2D2Emlhxo7YV
/uSTS+q4VuqU/C+pYsQalGa9Z42kSR2fj3+QjFz5oCW/XazSIzo=
=CNiT
-----END PGP SIGNATURE-----

--2JGdTg12ZKrKP4HO--