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
signature.asc
Description: PGP signature