> On Thu, Jul 18, 2024 at 02:56:20PM GMT, Michael Paquier wrote:
> On Tue, Jul 16, 2024 at 10:27:25AM +0900, Michael Paquier wrote:
> > Perhaps we should have a few more inline functions like
> > pgstat_get_entry_len() to retrieve the parts of the custom data in the
> > snapshot and shmem control structures for fixed-numbered stats. That
> > would limit what extensions need to know about
> > pgStatLocal.shmem->custom_data[] and
> > pgStatLocal.snapshot.custom_data[], which is easy to use incorrectly.
> > They don't need to know about pgStatLocal at all, either.
> >
> > Thinking over the weekend on this patch, splitting injection_stats.c
> > into two separate files to act as two templates for the variable and
> > fixed-numbered cases would be more friendly to developers, as well.
>
> I've been toying a bit with these two ideas, and the result is
> actually neater:
> - The example for fixed-numbered stats is now in its own new file,
> called injection_stats_fixed.c.
> - Stats in the dshash are at the same location, injection_stats.c.
> - pgstat_internal.h gains two inline routines called
> pgstat_get_custom_shmem_data and pgstat_get_custom_snapshot_data that
> hide completely the snapshot structure for extensions when it comes to
> custom fixed-numbered stats, see the new injection_stats_fixed.c that
> uses them.
Agree, looks good. I've tried to quickly sketch out such a fixed
statistic for some another extension, everything was fine and pretty
straightforward. One question, why don't you use
pgstat_get_custom_shmem_data & pgstat_get_custom_snapshot_data outside
of the injection points code? There seems to be a couple of possible
places in pgstats itself.