> > 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.
The proposal is not about validating that the data being read back
belongs to the same registered kind. It is about ensuring that if a
kind is not registered, we simply skip that portion of the
pgstat.stat file. It's a very narrow case, perhaps too narrow to
be worthwhile.
> 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).
Hmm, since the pgstat.stat file is managed by core and not by the
extension, I think this should be handled semi-automatically by
pgstats. Even with the checks you mention above, validating that we
are indeed loading the same kind will never be totally foolproof. I
think we can validate kinds better by adding some MAGIC NUMBER that is
set by the extension during registration and tracked in pgstats.stat.
If the kind ID
and MAGIC NUMBER (for example, FORMAT ID) match, then we have more of
a guarantee that this is the same extension. Also, this is probably a
good idea to support extension upgrades.
--
Sami
