Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <aS_QrGCgB7AMlSdU@paquier.xyz>
Date: Wed, 3 Dec 2025 14:16:54 +0800
Cc: Sami Imseih <samimseih@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <5EC13E39-B41E-464A-B382-0ACEB18A15BB@gmail.com>
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>
 <aS_QrGCgB7AMlSdU@paquier.xyz>
To: Michael Paquier <michael@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/5EC13E39-B41E-464A-B382-0ACEB18A15BB%40gmail.com>
Precedence: bulk


> On Dec 3, 2025, at 13:54, Michael Paquier <michael@paquier.xyz> wrote:
>=20
> 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 =
reading 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?
>=20
> 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.
>=20
>> * 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?
>=20
> Extension code would be responsible for ensuring that.
>=20
>> * 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?
>=20
> 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.
>=20
> 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?
>=20
>> * 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.

Yes, they of course can do, but that=E2=80=99s out of pgstat=E2=80=99s =
control. How can we ensure that?

>=20
>> 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

If we push down the responsibility into the extension code, then all =
extensions that want to enjoy the callbacks have to handle the same =
complexities of dealing with stat files, which sounds big duplicate =
efforts.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/