On Thu, Jun 13, 2024 at 05:35:49PM -0700, Noah Misch wrote:
> I think the attached covers all comments to date. I gave everything v3, but
> most patches have just a no-conflict rebase vs. v2. The exceptions are
> inplace031-inj-wait-event (implements the holding from that branch of the
> thread) and inplace050-tests-inj (updated to cooperate with inplace031). Much
> of inplace031-inj-wait-event is essentially s/Extension/Custom/ for the
> infrastructure common to the two custom wait event types.
Looking at inplace031-inj-wait-event..
The comment at the top of GetWaitEventCustomNames() requires an
update, still mentioning extensions.
GetWaitEventCustomIdentifier() is incorrect, and should return
"InjectionPoint" in the default case of this class name, no? I would
just pass the classID to GetWaitEventCustomIdentifier().
It is suboptimal to have pg_get_wait_events() do two scans of
WaitEventCustomHashByName. Wouldn't it be better to do a single scan,
returning a set of (class_name,event_name) fed to the tuplestore of
this SRF?
uint32
WaitEventExtensionNew(const char *wait_event_name)
{
+ return WaitEventCustomNew(PG_WAIT_EXTENSION, wait_event_name);
+}
+
+uint32
+WaitEventInjectionPointNew(const char *wait_event_name)
+{
+ return WaitEventCustomNew(PG_WAIT_INJECTIONPOINT, wait_event_name);
+}
Hmm. The advantage of two routines is that it is possible to control
the class IDs allowed to use the custom wait events. Shouldn't the
second routine be documented in xfunc.sgml?
wait_event_names.txt also needs tweaks, in the shape of a new class
name for the new class "InjectionPoint" so as it can be documented for
its default case. That's a fallback if an event ID cannot be found,
which should not be the case, still that's more correct than showing
"Extension" for all class IDs covered by custom wait events.
--
Michael