On Sat, May 25, 2024 at 07:52:02AM +0000, Bertrand Drouvot wrote:
> But I think that it is in a state that can be used to discuss the approach it
> is implementing (so that we can agree or not on it) before moving
> forward.
I have read through the patch to get an idea of how things are done,
and I am troubled by the approach taken (mentioned down by you), but
that's invasive compared to how pgstats wants to be transparent with
its stats kinds.
+ Oid objoid; /* object ID, either table or function
or tablespace. */
+ RelFileNumber relfile; /* relfilenumber for RelFileLocator. */
} PgStat_HashKey;
This adds a relfilenode component to the central hash key used for the
dshash of pgstats, which is something most stats types don't care
about. That looks like the incorrect thing to do to me, particularly
seeing a couple of lines down that a stats kind is assigned so the
HashKey uniqueness is ensured by the KindInfo:
+ [PGSTAT_KIND_RELFILENODE] = {
+ .name = "relfilenode",
FWIW, I have on my stack of patches something to switch the objoid to
8 bytes, actually, which is something that would be required for
pg_stat_statements as query IDs are wider than that and affect all
databases, FWIW. Relfilenodes are 4 bytes, okay still Robert has
proposed a couple of years ago a patch set to bump that to 56 bits,
change reverted in a448e49bcbe4. The objoid is also not something
specific to OIDs, see replication slots with their idx for example.
What you would be looking instead is to use the relfilenode as an
objoid and keep track of the OID of the original relation in each
PgStat_StatRelFileNodeEntry so as it is possible to know where a past
relfilenode was used? That makes looking back at the past relation's
elfilenodes stats more complicated as it would be necessary to keep a
list of the past relfilenodes for a relation, as well. Perhaps with
some kind of cache that maintains a mapping between the relation and
its relfilenode history?
--
Michael
