Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 25 Nov 2025 11:03:22 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Message-ID: <aSUOaqT53UBIYTKh@paquier.xyz>
References: 
 <CAA5RZ0s9SDOu+Z6veoJCHWk+kDeTktAtC-KY9fQ9Z6BJdDUirQ@mail.gmail.com>
 <aPmh7uoW6_EmxFHd@paquier.xyz>
 <CAA5RZ0ux07=E97mJ3JCy2jhs349NKzj7SPJNbDWZ11m9BDkTQQ@mail.gmail.com>
 <CAA5RZ0tnopAS4MCcoxXdJShYoGxYZgsoDXCjcie0Q-2TBPuJwA@mail.gmail.com>
 <aRJzdl_cSc3XHX85@paquier.xyz>
 <CAA5RZ0skT-uWwgrCg3Hj3AQaBbQdN0h72scD9GBLxH1nEKSNwQ@mail.gmail.com>
 <CAA5RZ0tH6ih3anQ=Lcwttm+8BURJ8VkBoy_9V8Ty73ZA1r384Q@mail.gmail.com>
 <aSOidfDnBT4lbtHp@paquier.xyz>
 <CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@mail.gmail.com>
 <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="ygdrJBdks8wPMJSw"
Content-Disposition: inline
In-Reply-To: 
 <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aSUOaqT53UBIYTKh%40paquier.xyz>
Precedence: bulk


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

On Mon, Nov 24, 2025 at 06:18:26PM -0600, Sami Imseih wrote:
> After second thought, I am not too thrilled with extending reset_all_cb
> to take responsibility for file cleanup, etc. I think it should just rema=
in
> used to reset stats only.
>=20
> I think the best way forward will be to introduce a callback to be used by
> custom kinds only. This callback will be responsible for cleaning up files
> and related resources at the end of the write stats, read stats, and disc=
ard
> stats paths. The callback will provide back to the extension a status
> (READ, WRITE, DISCARD) and the extension will know how to clean up the
> resources it created depending on the situation.

I guess that READ and WRITE are the cases that happen on success of
these respective operations.  DISCARD is the failure case when one of
these fail.

> So, unlike my original proposal, this puts more responsibility on the
> extension to track and clean up its files, but this seems like the best
> approach to take here.

That may be something we should do anyway.  It means that the modules
are responsible for the tracking the file(s) they open, still they
could also decide operations different than the backend for the main
pgstats file, optionally, depending on the state of the reads and
writes (aka success or failure of these).

> Also, I am now leaning towards creating a separate test module rather than
> trying to do too much unrelated testing in injection points. It is defini=
tely
> convenient to use injection points, but I think we can do better testing =
with
> a separate module. This module can also serve as an example for extension
> developers.

You are right that it may be cleaner this way.  Do you think that it
could make sense to move some of the existing "template" code of
injection_points there?

One part of the proposed patch that felt independent to me was the
renaming and publishing of the two write/read routines for the stats
files, so I have extracted that in your first patch to reduce the
blast, and applied that as it can also be useful on its own.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmklDmoACgkQnvQgOdby
QH1tMxAAm4NaZ1zFYE318+8R7p3zwMlZKGxPQn5IoewLmciygGC1Z9HHAKvFwRJm
KE8GTErxpVY3TF4eopIMWfOJ6RwvnhYzUp7OPIRaKi5nTxWEj2L3W7qSttxckDTu
t5NrTAQ6t3CURNg9L1H80FNZAdw5xSBA9ouV1aFSZK+lBwuB5EJey4t2SF9cQXt6
NzrcmImQHvZU00WwjRj5OKrhVq5uEm1McBjqiSsi+e6/rvfm8RSmMaAQ2DIyRlSr
9/LajyiNVxsuUVkmZHB7d2wAR2Zqb6X71WUjU81OeSp/QtvO1quQeKnTnCMkWhD+
mEgtLYeZjVEdBuYCsxajoHMODmIZB5Yp7yRmp40M+kjd5dVs2Yh3RHMExfj7C8MA
muERZy8o/p5c2OfCsVsGpasqnQBIJj/YJls+SD8Eb6RXemIsjCv65PvYfWTPqWxp
yUzWeygsgZ/OpYeZxLZZsfYnk4vQYdHdu7N9tSoM25gI6Eq5Ieaa4oIkjI2rw1th
J7uQZekXw3G9pc0urVUkT/h2CSecERhtGTksDiuX1/H/lcbuL3M8lYM8Mu7QkMgY
3G5fstnm1ICmmmC+/B3/46qCcT6bwpsZ/cqcRdTBlaW1wV4F7ADG2IaZBh4L5TCP
7k2iVOh/tVi57cFmdiurn1+FLVAmIwPcUcMww85yYCzaj1bTeko=
=KwZ2
-----END PGP SIGNATURE-----

--ygdrJBdks8wPMJSw--