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

Hi,

Thank you for your review.

>
> +       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.
>
The intention was to keep the implementation consistent across all
versions of injection_points_attach(). I agree that the conditional
implementation
is only feasible if the library defining the new function also supports the same
structure as injection_points.c. Therefore, I am okay with not adding
this support
for custom functions, since it would not be complete without the new function
being able to read the same structure.

> 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.
>

This seems helpful for situations where each module needs to provide its
own custom data to the injection point.  This idea is implemented in the
attached patch.

> 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.

OK, it makes sense to leave it out of this function for now. Since
pgstat_create_inj() currently only tracks the number of runs, it also
depends on any callback using the appropriate pgstat_report_* API
from the injection_point module. Without this, setting up the stats
infrastructure wouldn't be useful.

PFA the updated and rebased patch.

Thank you,
Rahila Syed