On 2024-Aug-22, Michael Paquier wrote:
> On Wed, Aug 21, 2024 at 01:55:06PM -0400, Alvaro Herrera wrote:
> > Also, maybe it'd make sense for stats to be globally enabled, and that
> > only the tests that require it would disable them? (It's probably easy
> > enough to have a value "injection_points.stats=auto" which means, if the
> > module is loaded in shared_preload_libraries them set stats on,
> > otherwise turn them off.)
>
> I'm not sure that we need to get down to that until somebody has a
> case where they want to rely on stats of injection points for their
> stuff. At this stage, I only want the stats to be enabled to provide
> automated checks for the custom pgstats APIs, so disabling it by
> default and enabling it only in the stats test of the module
> injection_points sounds kind of enough to me for now.
Oh! I thought the stats were useful by themselves. That not being the
case, I agree with simplifying; and the other ways to enhance this point
might not be necessary for now.
> > Or give the _LOAD() macro a boolean argument to
> > indicate whether to collect stats for that injection point or not.
>
> Sticking some knowledge about the stats in the backend part of
> injection points does not sound like a good idea to me.
You could flip this around: have the bool be for "this injection point
is going to be invoked inside a critical section". Then core code just
needs to tell the injection points module what core code does, and it's
injection_points that decides what to do with that information.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Crear es tan difícil como ser libre" (Elsa Triolet)
