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)
