Re: Support to define custom wait events for extensions

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Cc: Tristan Partin <tristan(at)neon(dot)tech>, andres(at)anarazel(dot)de, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Support to define custom wait events for extensions
Date: 2023-07-28 01:06:17
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jul 27, 2023 at 06:29:22PM +0900, Masahiro Ikeda wrote:
> I suspect that I forgot to specify "volatile" to the variable
> for the spinlock.

+ if (!IsUnderPostmaster)
+ {
+ /* Allocate space in shared memory. */
+ waitEventExtensionCounter = (WaitEventExtensionCounter *)
+ ShmemInitStruct("waitEventExtensionCounter", WaitEventExtensionShmemSize(), &found);
+ if (found)
+ return;

I think that your error is here. WaitEventExtensionShmemInit() is
forgetting to set the pointer to waitEventExtensionCounter for
processes where IsUnderPostmaster is true, which impacts things not
forked like in -DEXEC_BACKEND (the crash is reproducible on Linux with
-DEXEC_BACKEND in CFLAGS, as well). The correct thing to do is to
always call ShmemInitStruct, but only initialize the contents of the
shared memory area if ShmemInitStruct() has *not* found the shmem

WaitEventExtensionNew() could be easily incorrectly used, so I'd
rather add a LWLockHeldByMeInMode() on AddinShmemInitLock as safety
measure. Perhaps we should do the same for the LWLocks, subject for a
different thread..

+ int newalloc;
+ newalloc = pg_nextpower2_32(Max(8, eventId + 1));

This should be a uint32.

+ if (eventId >= WaitEventExtensionNamesAllocated ||
+ WaitEventExtensionNames[eventId] == NULL)
+ return "extension";
That's too close to the default of "Extension". It would be cleaner
to use "unknown", but we've been using "???" as well in many default
paths where an ID cannot be mapped to a string, so I would recommend
to just use that.

I have spent more time polishing the docs and the comments. This v9
looks in a rather committable shape now with docs, tests and core
routines in place.

Attachment Content-Type Size
v9-0001-Support-custom-wait-events-for-wait-event-type-Ex.patch text/x-diff 21.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-07-28 01:50:50 Re: Fwd: BUG #18016: REINDEX TABLE failure
Previous Message Peter Smith 2023-07-27 23:57:06 Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication