Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 3 Dec 2025 14:54:52 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Message-ID: <aS_QrGCgB7AMlSdU@paquier.xyz>
References: 
 <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>
 <aSUOaqT53UBIYTKh@paquier.xyz>
 <CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com>
 <4FEEC2B8-D9CF-4BD0-AA7E-44CE3E27ADD8@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="uxSbyhfrPmVKHRC6"
Content-Disposition: inline
In-Reply-To: <4FEEC2B8-D9CF-4BD0-AA7E-44CE3E27ADD8@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aS_QrGCgB7AMlSdU%40paquier.xyz>
Precedence: bulk


--uxSbyhfrPmVKHRC6
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 03, 2025 at 01:41:44PM +0800, Chao Li wrote:
> Thanks for the patch, I do think the feature will be useful. After readin=
g the patch, I got a concern on the design:
>=20
> This patch provides callbacks that requests (also allows) custom
> extensions to write stat files on their own behalf, which I think
> it=E2=80=99s unsafe. The problems coming out to my head includes:
>=20
> * An extension can write to any where on the storage, that what
> * about it writes to /tmp and the files are deleted by other process
> * or by a user manually incidentally?

I mean, just don't do that.  It's up to the extension developer to
decide what is safe or not, within the scope of the data folder.

> * pgstat has a pattern of writing files like: writing to tmp file
> * first, then durable_rename(), how to ensure extensions to do the
> * same pattern? Without this pattern, how to ensure reliability of
> * stat files?

Extension code would be responsible for ensuring that.

> * In the current path, pgstat performs its own write, then call
> * callbacks. What about if a callback fails? Will that leave pgstat
> * in a stale state?

For the write state, end_extra_stats() would take care of that.  It
depends on what kind of errors you would need to deal with, but as
proposed the patch would offer the same level of protection for the
writes of the stats, where we'd check for an error on the fd saved by
an extension for an extra file.

I think that you have a fair point about the stats read path though,
shouldn't we make the callback from_serialized_extra_stats() return a
state to be able to trigger a full-scale cleanup, at least?

> * As extensions own file creation and deletion, in some case, staled
> * file might be left on storage, who will be responsible for
> * cleaning up them?=20

The extension should be able to handle that, I guess.

> Given the goal of the feature is to allow extensions to serialize
> custom data, the callback should just return serialized/deserialized
> data, maybe together with some metadata, then pgstat should be
> responsible for writing the data. In other words, IMO, pgstat should
> always own stat files.=20

That's where my view of the matter differs, actually, pushing down the
responsibility into the extension code itself.  A key argument,
mentioned upthread, is that the file paths could depend on the stats
entry *keys*, which may not be known in advance when beginning the
flush of the stats.  Think about per-database file stats, or just
some per-object file stats, for example, which is an option that would
matter so as we do not bloat the main pgstats file.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmkv0KwACgkQnvQgOdby
QH077Q//ZXF+Pvj2K1UsmoIpdEAV3iWPJIiFT+bd3C52JydO04ked1XMhsTYdOWj
AVqHqAnii+jfmtF8kZvTr3Cyjk+Y0tAz+1KaZ4HywVK1V07Isqojh93VsB5f3ggL
7HQjL8VvaGH8k3FBIO3QOWEcARGe5g7wNHRTmfHUUZfrhg46A0eLFAVoID56mP3d
XUWDJWTNKfzsM5qqXbu1F6KyduqFubUphU12W5VmmtkmHJiOhjn+X9LXimtxXuRQ
Kh0j8xrm7jRH+HFeV5/jZrThnZNBPpBzAtpAkHo2omVIKiTZyNLGuqGJquoLn0qI
gTjtvEgcu/WPwrxcdOhI82pGHV7wDFqObjyv0mTmtZ79NbcUXbD9sAA5mFtV4o9J
mS6PsPSRFnfq1O7SsNmqoJfdDHrV6EyUr+Nntm+QdS8/ptiVsYeoWP1kNl6Sr00C
IeK3ibKFTTqi2zXUU4Sddnk10pqtctM3TtvDvG+2hfSypdFD4SMyG+EsYXM3ZPjX
KuvdIgpcFh9L4xizRY4brzApFvMk+mmMeh3Pd+GTz0+eU4WCCNK0HBKAye5NaLBv
ZeHAhvl7jM3VrztJ+bGv/cy7YVbUNrlz0gSPA7GUUNuMQXX0HZV097eAOc6dDnDB
NsEbVG0J38CHEKtTVcZ8r4tRKLLyJhF4Ce0+h4Wze6uT11KupAk=
=nyYU
-----END PGP SIGNATURE-----

--uxSbyhfrPmVKHRC6--