On Mon, Jun 24, 2024 at 11:03:09AM -0400, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
> > ... I can't do that, because InjectionPointRun() requires a PGPROC
> > entry, because it uses an LWLock. That also makes it impossible to use
> > injection points in the postmaster. Any chance we could allow injection
> > points to be triggered without a PGPROC entry? Could we use a simple
> > spinlock instead?
That sounds fine to me. If calling hash_search() with a spinlock feels too
awful, a list to linear-search could work.
> > With a fast path for the case that no injection points
> > are attached or something?
>
> Even taking a spinlock in the postmaster is contrary to project
> policy. Maybe we could look the other way for debug-only code,
> but it seems like a pretty horrible precedent.
If you're actually using an injection point in the postmaster, that would be
the least of concerns. It is something of a concern for running an injection
point build while not attaching any injection point. One solution could be a
GUC to control whether the postmaster participates in injection points.
Another could be to make the data structure readable with atomics only.
> Given your point that the existing locking is just a fig leaf
> anyway, maybe we could simply not have any? Then it's on the
> test author to be sure that the injection point won't be
> getting invoked when it's about to be removed.
That's tricky with injection_points_set_local() plus an injection point at a
frequently-reached location. It's one thing to control when the
injection_points_set_local() process reaches the injection point. It's too
hard to control all the other processes that just reach the injection point
and conclude it's not for their PID.
> Or just rip
> out the removal capability, and say that once installed an
> injection point is there until postmaster shutdown (or till
> shared memory reinitialization, anyway).
That could work. Tests do need a way to soft-disable, but it's okay with me
if nothing can reclaim the resources.
