Re: Injection point locking
Noah Misch <noah@leadboat.com>
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-06-25T02:25:37Z
Lists: pgsql-hackers
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.
Commits
-
Improve locking around InjectionPointRun()
- d85fc4be11b3 17.0 landed