Re: Injection point locking

2024-07-09 Thread Heikki Linnakangas
On 09/07/2024 08:16, Michael Paquier wrote: typedef struct InjectionPointCacheEntry { charname[INJ_NAME_MAXLEN]; +intslot_idx; +uint64generation; charprivate_data[INJ_PRIVATE_MAXLEN]; InjectionPointCallback callback; }

Re: Injection point locking

2024-07-08 Thread Michael Paquier
On Mon, Jul 08, 2024 at 04:21:37PM +0300, Heikki Linnakangas wrote: > I came up with the attached. It replaces the shmem hash table with an array > that's scanned linearly. On each slot in the array, there's a generation > number that indicates whether the slot is in use, and allows detecting >

Re: Injection point locking

2024-07-08 Thread Michael Paquier
On Mon, Jul 08, 2024 at 10:17:49AM -0400, Tom Lane wrote: > Heikki Linnakangas writes: >> Note that until we actually add an injection point to a function that >> runs in the postmaster, there's no risk. If we're uneasy about that, we >> could add an assertion to InjectionPointRun() to prevent

Re: Injection point locking

2024-07-08 Thread Tom Lane
Heikki Linnakangas writes: > Note that until we actually add an injection point to a function that > runs in the postmaster, there's no risk. If we're uneasy about that, we > could add an assertion to InjectionPointRun() to prevent it from running > in the postmaster, so that we don't cross

Re: Injection point locking

2024-07-08 Thread Heikki Linnakangas
On 25/06/2024 05:25, Noah Misch wrote: On Mon, Jun 24, 2024 at 11:03:09AM -0400, Tom Lane wrote: Heikki Linnakangas 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

Re: Injection point locking

2024-06-27 Thread Michael Paquier
On Wed, Jun 26, 2024 at 10:56:12AM +0900, Michael Paquier wrote: > That's true, we could delay the release of the lock to happen just > before a callback is run. I am not sure what else we can do for the postmaster case for now, so I've moved ahead with the concern regarding the existing locking

Re: Injection point locking

2024-06-25 Thread Michael Paquier
On Tue, Jun 25, 2024 at 09:10:06AM -0700, Noah Misch wrote: > I think your last sentence is what Heikki is saying should happen, and I > agree. Yes, it matters. As written, InjectionPointRun() could cache an > entry_by_name->function belonging to a different injection point. That's true, we

Re: Injection point locking

2024-06-25 Thread Noah Misch
On Tue, Jun 25, 2024 at 11:14:57AM +0900, Michael Paquier wrote: > On Mon, Jun 24, 2024 at 01:29:38PM +0300, Heikki Linnakangas wrote: > > InjectionPointRun() acquires InjectionPointLock, looks up the hash entry, > > and releases the lock: > > > > > LWLockAcquire(InjectionPointLock, LW_SHARED);

Re: Injection point locking

2024-06-24 Thread Michael Paquier
On Mon, Jun 24, 2024 at 11:03:09AM -0400, Tom Lane wrote: > 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.

Re: Injection point locking

2024-06-24 Thread Noah Misch
On Mon, Jun 24, 2024 at 11:03:09AM -0400, Tom Lane wrote: > Heikki Linnakangas 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

Re: Injection point locking

2024-06-24 Thread Michael Paquier
On Mon, Jun 24, 2024 at 01:29:38PM +0300, Heikki Linnakangas wrote: > InjectionPointRun() acquires InjectionPointLock, looks up the hash entry, > and releases the lock: > > > LWLockAcquire(InjectionPointLock, LW_SHARED); > > entry_by_name = (InjectionPointEntry *) > >

Re: Injection point locking

2024-06-24 Thread Tom Lane
Heikki Linnakangas 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?

Injection point locking

2024-06-24 Thread Heikki Linnakangas
InjectionPointRun() acquires InjectionPointLock, looks up the hash entry, and releases the lock: LWLockAcquire(InjectionPointLock, LW_SHARED); entry_by_name = (InjectionPointEntry *) hash_search(InjectionPointHash, name,