Re: [Proposal] Adding callback support for custom statistics kinds

> On Dec 3, 2025, at 04:58, Sami Imseih <samimseih@gmail.com> wrote:
>
>>> 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 definitely
>>> 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?
>
> By "template" code, do you mean Something like?
>
> include/utils/custom_statkinds.h
> backend/utils/misc/custom_statkinds.c
>
> Where the template code here is PgStat_kind definition, callbacks, etc. for
> injection_points or the new test module that is using a custom kind.
>
> A few benefits I see for this is we can point extension developers to
> this as an example in [0] and we are also maintaining the kind ids in
> a single place. These may not be strong points, but may be worth while.
>
> v2 attached is something that may be closer to what we've been discussing
>
> v2-0001 are much simplified changes to pgstat.c that simply invoke the callbacks
> and all the work is on the extension to implement what it needs to do.
> This includes
> a callback at the end of WRITE, READ, DISCARD with a flag passed to the caller
> so they can perform the necessary clean-up actions.
>
> v2-0002 implements a new test module that tests mainly that the recovery,
> clean and crash, are working as expected.
>
> I created a new tap test for this which performs a test similar to what is
> done in recovery/029_stats_restart.pl. I could merge the new test there, but
> I am reluctant to add a dependency on a new module to recovery. What
> do you think?
>
> [0] https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-ADDIN-CUSTOM-CUMULATIVE-STATISTICS
>
>
> --
> Sami Imseih
> Amazon Web Services (AWS)
> <v2-0002-Tests-for-custom-stat-kinds.patch><v2-0001-pgstat-support-custom-serialization-files-and-cal.patch>

Thanks for the patch, I do think the feature will be useful. After reading the patch, I got a concern on the design:

This patch provides callbacks that requests (also allows) custom extensions to write stat files on their own behalf,
whichI think it’s unsafe. The problems coming out to my head includes: 

* An extension can write to any where on the storage, that what about it writes to /tmp and the files are deleted by
otherprocess or by a user manually incidentally? 
* pgstat has a pattern of writing files like: writing to tmp file first, then durable_rename(), how to ensure
extensionsto do the same pattern? Without this pattern, how to ensure reliability of stat files? 
* In the current path, pgstat performs its own write, then call callbacks. What about if a callback fails? Will that
leavepgstat in a stale state? 
* As extensions own file creation and deletion, in some case, staled file might be left on storage, who will be
responsiblefor cleaning up them? 

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

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