Feedback-ID: ia2694551:Fastmail
Date: Thu, 22 Aug 2024 10:36:38 -0400
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>, 
	vignesh C <vignesh21@gmail.com>, Andrew Borodin <amborodin86@gmail.com>, 
	Yura Sokolov <y.sokolov@postgrespro.ru>, Andres Freund <andres@anarazel.de>, 
	Thomas Munro <thomas.munro@gmail.com>, Gilles Darold <gilles@darold.net>, 
	Alexander Korotkov <aekorotkov@gmail.com>, Daniel Gustafsson <daniel@yesql.se>, 
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers mailing list <pgsql-hackers@postgresql.org>
Subject: Re: MultiXact\SLRU buffers configuration
Message-ID: <202408221436.o6girevg72sx@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZsaGaqKV7ghWhG57@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/202408221436.o6girevg72sx%40alvherre.pgsql>
Precedence: bulk

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)