public inbox for [email protected]  
help / color / mirror / Atom feed
From: Sami Imseih <[email protected]>
To: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Mon, 24 Nov 2025 18:18:26 -0600
Message-ID: <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com> (raw)
In-Reply-To: <CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@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>

> After re-reading your earlier suggestions, this sounds like a third
> callback that is used for file cleanup, and this callback could be
> the existing reset_all_cb. Also, instead of reset_all_cb being called
> during pgstat_reset_after_failure, it can be called during the success
> case, i.e, a new pgstat_reset_after_success. reset_all_cb also
> carries a status argument so the extension knows what to do
> in the case of success or failure.

> This also means we need to also update all existing callbacks to
> do work in the failed status.

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.

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.

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.

what do you think?

--
Sami Imseih
Amazon Web Services (AWS)





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: <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>

* 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