Hi,
On 2025-04-14 16:46:22 +0900, Michael Paquier wrote:
> While reading the AIO code I have noticed that the way it uses
> injection points is limited by the fact that we don't have support for
> runtime parameters in the existing injection point facility.
Yep.
> The code is shaped with two set/get functions that set a static parameter
> that the injection callback would reuse internally, using
> pgaio_inj_io_get(), and pgaio_io_call_inj() and a static
> pgaio_inj_cur_handle. Relying on a static variable for that is not a good
> idea, IMO, even if the stack is reset with a TRY/CATCH block on error in the
> callback run.
It's not great, I agree. I was frankly rather surprised to see that injection
points don't support anything to pass parameters, since, without that, you
really can't actually inject different results or anything reliably.
> We are still in early April, and I'd like to propose a cleanup of the
> AIO code on HEAD, even if we are post-feature freeze, to not release
> this new code in this state, implying an open item. Please note that
> I'm OK to take responsibility for this patch set at the end, reviews
> are welcome.
I'm pretty agnostic about this happening for 18 or not. If we can do it,
good, but if not, the impact of needing to use static variable supporting
injection points is really rather small.
I'd say that the fact that injection variables are really hard to use in
critical sections, requiring to have attached to the injection point
beforehand, is worse.
> Anyway, I have spent some time with my mind on that, and finished
> with the attached patch set:
> - 0001 is the addition of runtime parameters in the backend code. I
> have made the choice of extending the existing INJECTION_POINT() and
> INJECTION_POINT_CACHED() instead of introducing new macros. That's a
> matter of taste, perhaps, but increasing the number of these macros
> leads to a confusing result. This one is more invasive, but that's OK
> for me.
I can see arguments for going either way on that one.
> The code is shaped so as we can rely on InjectionPointCallback to define the
> shape of a callback.
I don't know what this means though?
Greetings,
Andres Freund
