On Mon, Oct 20, 2025 at 07:54:09PM -0500, Sami Imseih wrote:
> The more worrying case is if the struct of this other kind has
> the same length, and the data ends up being read back
> into the wrong fields.
More to the point. If I were to put defenses when reading back a
stats file that includes custom stats kinds before reading the file
entries and checking if its contents are OK to be used by the server,
validating the size of the on-disk entries is not sufficient. We
should write down and cross-check more data when reading them back
with what has been loaded by the postmaster:
- Name of custom callback, that should match.
- Its reserved ID.
- Written size on disk, to ensure that what is read from the file is
OK to load.
- Likely if it is a fixed_amount or not, not really mandatory IMO.
Then, based on if these contents match with what has been loaded from
s_p_l, we could issue a harder failure that prevents startup and not
throw about the stats file. If one needs to ditch some stats, they
would still need to load a dummy version of the custom stats kind they
want to drop, dropping the entries on read. That's still an existing
requirement, or have a tool that's able to read, filter and rewrite an
existing stats file (refactoring pgstat_shmem.c to expose some of its
routines for this purpose has been on my TODO list of things for some
time).
The existing implementation is a choice of simplicity, prioritizing
availability, FWIW, to keep the contents of the on-disk file simpler.
Being able for an extension developer to drop stats in an existing
file would still need to be handled on the developer side. So I am
not really convinced that this is something we need to treat that as a
problem in search of a solution.
--
Michael
