Re: Injection point locking

From: Noah Misch <noah(at)leadboat(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Injection point locking
Date: 2024-06-25 02:25:37
Message-ID: 20240625022537.0f.nmisch@google.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 24, 2024 at 11:03:09AM -0400, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka(at)iki(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2024-06-25 02:28:37 Re: Injection point locking
Previous Message Zhijie Hou (Fujitsu) 2024-06-25 02:21:25 RE: New standby_slot_names GUC in PG 17