Horton

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

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-11-25T02:03:22Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix const correctness in pgstat data serialization callbacks

  2. test_custom_stats: Add tests with read/write of auxiliary data

  3. Allow cumulative statistics to read/write auxiliary data from/to disk

  4. test_custom_stats: Test module for custom cumulative statistics

  5. injection_points: Remove portions related to custom pgstats

  6. Rename routines for write/read of pgstats file

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 remain
> used to reset stats only.
> 
> 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 discard
> 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 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?

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