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
signature.asc
Description: PGP signature
