On Sat, Apr 06, 2024 at 10:34:46AM +0500, Andrey M. Borodin wrote:
> I find name of the function "injection_points_local()" strange,
> because there is no verb in the name. How about
> "injection_points_set_local"?
That makes sense.
> I'm not sure if we should refactor anything here, but
> InjectionPointSharedState has singular name, plural wait_counts and
> singular condition.
> InjectionPointSharedState is already an array of injection points,
> maybe let's add there optional pid instead of inventing separate
> array of pids?
Perhaps we could unify these two concepts, indeed, with a "kind" added
to InjectionPointCondition. Now waits/wakeups are a different beast
than the conditions that could be assigned to a point to filter if it
should be executed. More runtime conditions coming immediately into
my mind, that could be added to this structure relate mostly to global
objects, like:
- Specific database name and/or OID.
- Specific role(s).
So that's mostly cross-checking states coming from miscadmin.h for
now.
> Can we set global point to 'notice', but same local to 'wait'? Looks
> like now we can't, but allowing to do so would make code simpler.
You mean using the name point name with more than more callback? Not
sure we'd want to be able to do that. Perhaps you're right, though,
if there is a use case that justifies it.
> Besides this opportunity to simplify stuff, both patches looks good
> to me.
Yeah, this module can be always tweaked more if necessary. Saying
that, naming the new thing "condition" in InjectionPointSharedState
felt strange, as you said, because it is an array of multiple
conditions.
For now I have applied 997db123c054 to make the GIN tests with
injection points repeatable as it was an independent issue, and
f587338dec87 to add the local function pieces.
Attached is the last piece to switch the GIN test to use local
injection points. 85f65d7a26fc should maintain the buildfarm at bay,
but I'd rather not take a bet and accidently freeze the buildfarm as
it would impact folks who aim at getting patches committed just before
the finish line. So I am holding on this one for a few more days
until we're past the freeze and the buildfarm is more stable.
--
Michael