Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 9 Dec 2025 13:45:17 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Message-ID: <aTepXZ97PsXpuywI@paquier.xyz>
References: <4FEEC2B8-D9CF-4BD0-AA7E-44CE3E27ADD8@gmail.com>
 <aS_QrGCgB7AMlSdU@paquier.xyz>
 <5EC13E39-B41E-464A-B382-0ACEB18A15BB@gmail.com>
 <CAA5RZ0sJgO6GAwgFxmzg9MVP=rM7Us8KKcWpuqxe-f5qxmpE0g@mail.gmail.com>
 <aTIV8qfcIkht1Hgv@paquier.xyz>
 <CAA5RZ0sG2RUKg=OLY+6-e4q=X9rsLfK3pKn03d=RZQppEDR=Bg@mail.gmail.com>
 <aTZxqzPWr0wMVLq2@paquier.xyz>
 <CAA5RZ0tST1=piXSQwRn2y+YB-VwS+Dtr0Wk27Dw7ED+Xp6qSXQ@mail.gmail.com>
 <A5D8954F-2E20-4BF5-BA0E-79FDC06A632F@gmail.com>
 <CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="CPPyWJl1iIp+4DWY"
Content-Disposition: inline
In-Reply-To: 
 <CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aTepXZ97PsXpuywI%40paquier.xyz>
Precedence: bulk


--CPPyWJl1iIp+4DWY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 08, 2025 at 09:57:15PM -0600, Sami Imseih wrote:
> The way v5 is dealing with a deserialize failure is that when
> it goes to error, the pgstat_reset_after_failure() will reset the
> stats for all kinds, since pgstat_drop_all_entries() is called
> during that call. So there is nothing for an extension to have
> to do on its own. The extension will then clean-up resources
> at the end when  all the kinds are iterated over and
> kind_info->end_extra_stats(STATS_READ) is called for each
> kind.
>=20
> Let me know if I'm still missing something?

It seems to me that you are missing nothing here, and that Chao has
missed the fact that the end of pgstat_read_statsfile() does a "goto
done", meaning that we would take a round of
end_extra_stats(STATS_READ) to do all the cleanup after resetting all
the stats.  That's what I would expect.

+static inline bool pgstat_check_extra_callbacks(PgStat_Kind kind);
[...]
@@ -645,6 +656,13 @@ pgstat_initialize(void)
+	/* Check a kind's extra-data callback setup */
+	for (PgStat_Kind kind =3D PGSTAT_KIND_BUILTIN_MIN; kind <=3D PGSTAT_KIND_=
BUILTIN_MAX; kind++)
+		if (!pgstat_check_extra_callbacks(kind))
+			ereport(ERROR,
+					errmsg("incomplete extra serialization callbacks for stats kind %d",
+						   kind));

Why does this part need to run each time a backend initializes its
access to pgstats?  Shouldn't this happen only once when a stats kind
is registered?  pgstat_register_kind() should be the only code path
that does such sanity checks.

By the way, checking that to_serialized_extra_stats and
kind_info->from_serialized_extra_stats need to be both defined is
fine as these are coupled together, but I am not following the reason
why end_extra_stats would need to be included in the set?  For
example, a stats kind could decide to add some data to the main
pgstats file without creating extra files, hence they may not need to
define end_extra_stats.
--
Michael

--CPPyWJl1iIp+4DWY
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmk3qV0ACgkQnvQgOdby
QH2fmg//VaY2eJr7GTdnXTMo72MoZT26Eh+2xiMPGRIn9tF0AXzM4XG0CHhabnTb
7dSsAo89HznmpcRim144HErB9x+CzseCvedtCQEOh0CeG5Z3fVPACkG7NAvPLUe4
xLwYj1T2Gr2aBKtns5IwucQTp3TV5lVRRQ/o72qeACAPOZ+wix7q5FK7vz2HwI3Q
P/1Tp1xxCBChPXTxYMey/DUZOhFMwtjNyWRy+wcVIRxuk/tI9V3vSg9sSMBl0lQu
YOUfh8QkGsyNLGn3M86wM669f9FpYqS1OZvryjshUZF9pxO/KLYyc9N4pJt0Y+bw
G1HR7N6o19mzt7K63TAR2zlSKn7sgAnCPKnTnoxhWvq9bnUbFWUj7GGBrwlqr71w
J7TnVugYgt93gNxoX29S2NlPiJgDFpumr1S6s5IHelkdrZwTbZWtSwViwVC1ZNiE
6JErBcz9FRydhUotRfvS/44SoVtmFftjoNUHPvJFjSF4y7IgwPkjHpo7y6If/guX
gzxnPuxKrOzNX1MqZdVAird2nIaIBDIRnA1PQzbTOgKk3U/kprh5FJwlrfr1zq/V
R5qCjDscDsQ3gge6kC4E+zJU8LOeAusYUaBnO/2xj0CuZwU1yzs7JClvfqKQFIiF
0kqYMz7UoUXoGE/lId5/M9IY9k9R3mRSzYjUaAGhEkxXhqVhloo=
=y9CZ
-----END PGP SIGNATURE-----

--CPPyWJl1iIp+4DWY--