public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Sami Imseih <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Tue, 9 Dec 2025 13:45:17 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAA5RZ0sJgO6GAwgFxmzg9MVP=rM7Us8KKcWpuqxe-f5qxmpE0g@mail.gmail.com>
<[email protected]>
<CAA5RZ0sG2RUKg=OLY+6-e4q=X9rsLfK3pKn03d=RZQppEDR=Bg@mail.gmail.com>
<[email protected]>
<CAA5RZ0tST1=piXSQwRn2y+YB-VwS+Dtr0Wk27Dw7ED+Xp6qSXQ@mail.gmail.com>
<[email protected]>
<CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
On Mon, Dec 08, 2025 at 09:57:15PM -0600, Sami Imseih wrote:
> The way v5 is dealing with a deserialize failure is that when
> it goes to error, the pgstat_reset_after_failure() will reset the
> stats for all kinds, since pgstat_drop_all_entries() is called
> during that call. So there is nothing for an extension to have
> to do on its own. The extension will then clean-up resources
> at the end when all the kinds are iterated over and
> kind_info->end_extra_stats(STATS_READ) is called for each
> kind.
>
> Let me know if I'm still missing something?
It seems to me that you are missing nothing here, and that Chao has
missed the fact that the end of pgstat_read_statsfile() does a "goto
done", meaning that we would take a round of
end_extra_stats(STATS_READ) to do all the cleanup after resetting all
the stats. That's what I would expect.
+static inline bool pgstat_check_extra_callbacks(PgStat_Kind kind);
[...]
@@ -645,6 +656,13 @@ pgstat_initialize(void)
+ /* Check a kind's extra-data callback setup */
+ for (PgStat_Kind kind = PGSTAT_KIND_BUILTIN_MIN; kind <= PGSTAT_KIND_BUILTIN_MAX; kind++)
+ if (!pgstat_check_extra_callbacks(kind))
+ ereport(ERROR,
+ errmsg("incomplete extra serialization callbacks for stats kind %d",
+ kind));
Why does this part need to run each time a backend initializes its
access to pgstats? Shouldn't this happen only once when a stats kind
is registered? pgstat_register_kind() should be the only code path
that does such sanity checks.
By the way, checking that to_serialized_extra_stats and
kind_info->from_serialized_extra_stats need to be both defined is
fine as these are coupled together, but I am not following the reason
why end_extra_stats would need to be included in the set? For
example, a stats kind could decide to add some data to the main
pgstats file without creating extra files, hence they may not need to
define end_extra_stats.
--
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], [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