public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Sami Imseih <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Tue, 25 Nov 2025 11:03:22 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
References: <CAA5RZ0s9SDOu+Z6veoJCHWk+kDeTktAtC-KY9fQ9Z6BJdDUirQ@mail.gmail.com>
<[email protected]>
<CAA5RZ0ux07=E97mJ3JCy2jhs349NKzj7SPJNbDWZ11m9BDkTQQ@mail.gmail.com>
<CAA5RZ0tnopAS4MCcoxXdJShYoGxYZgsoDXCjcie0Q-2TBPuJwA@mail.gmail.com>
<[email protected]>
<CAA5RZ0skT-uWwgrCg3Hj3AQaBbQdN0h72scD9GBLxH1nEKSNwQ@mail.gmail.com>
<CAA5RZ0tH6ih3anQ=Lcwttm+8BURJ8VkBoy_9V8Ty73ZA1r384Q@mail.gmail.com>
<[email protected]>
<CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@mail.gmail.com>
<CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
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
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (47+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox