Re: Extend injection_points_attach() to accept a user-defined function

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Rahila Syed <rahilasyed90(at)gmail(dot)com>
Cc: Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extend injection_points_attach() to accept a user-defined function
Date: 2025-11-05 03:02:22
Message-ID: aQq-PhPO4khzmYDt@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 04, 2025 at 02:17:44PM +0530, Rahila Syed wrote:
> PFA a rebased patch that contains the test.
> The tests use the newly added SQL function to attach the injection_notice
> function to an injection point

+ if (injection_point_local)
+ {
+ condition.type = INJ_CONDITION_PID;
+ condition.pid = MyProcPid;
+ }

Hmm. Is there a point in registering a condition that's linked to
the shared library injection_points? The automatic drop is kind of
nice to have, I guess, but it gives the illusion that an attached
callback will not be run. However, a callback from an entirely
different library *will* run anyway because it cannot look at the
condition registered, or the other library has an equivalent able to
treat a local condition with the same format and same structure as
what's in injection_points.c.

Different idea: we could allow one to pass a bytea that could be given
directly to InjectionPointAttach()? Without a use-case, I cannot get
much excited about that yet, but if someone has a use for it, why not.

Maybe we should also discard the pgstat_create_inj() call in this
path? The existing injection_points_detach() would be able to deal
with a point attached with a callback from a different library anyway.
Keeping pgstat_report_inj_fixed() feels OK in this new attach path.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2025-11-05 03:47:37 Re: Fix comments for ChangeVarNodes() and related functions
Previous Message jian he 2025-11-05 02:48:21 Re: Adding REPACK [concurrently]