Re: Support to define custom wait events for extensions

On Mon, Jul 31, 2023 at 03:53:27PM +0900, Masahiro Ikeda wrote:
> I think the order in which they are mentioned should be matched. I mean that
> -     so an <literal>LWLock</literal> or <literal>Extension</literal> wait
> +     so an <literal>Extension</literal> or <literal>LWLock</literal> wait

Makes sense.

> 	/* This should only be called for user-defined wait event. */
> 	if (eventId < NUM_BUILTIN_WAIT_EVENT_EXTENSION)
> 		ereport(ERROR,
> 				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> 				errmsg("invalid wait event ID %u", eventId));
> 
> I was just wondering if it should also check the eventId
> that has been allocated though it needs to take the spinlock
> and GetWaitEventExtensionIdentifier() doesn't take it into account.

What kind of extra check do you have in mind?  Once WAIT_EVENT_ID_MASK
is applied, we already know that we don't have something larger than
PG_UNIT16_MAX, or perhaps you want to cross-check this number with
what nextId holds in shared memory and that we don't have a number
between nextId and PG_UNIT16_MAX?  I am not sure that we need to care
much about that this much in this code path, and I'd rather avoid
taking an extra time the spinlock just for a cross-check.

Attaching a v11 based on Bharath's feedback and yours, for now.  I
have also applied the addition of the two masking variables in
wait_event.c separately with 7395a90.
--
Michael