Re: Support to define custom wait events for extensions
От | Bharath Rupireddy |
---|---|
Тема | Re: Support to define custom wait events for extensions |
Дата | |
Msg-id | CALj2ACVq=BogP=iQ0C=Pjzk4+rSTaKZU_+fdMRk4YyG8B60gAQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Support to define custom wait events for extensions (Michael Paquier <michael@paquier.xyz>) |
Ответы |
Re: Support to define custom wait events for extensions
(Michael Paquier <michael@paquier.xyz>)
|
Список | pgsql-hackers |
On Fri, Jul 28, 2023 at 6:36 AM Michael Paquier <michael@paquier.xyz> wrote: > > 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. Thanks. Here are some comments on v9 patch: 1. - so an <literal>LWLock</literal> wait event might be reported as - just <quote><literal>extension</literal></quote> rather than the - extension-assigned name. + if the extension's library is not loaded; so a custom wait event might + be reported as just <quote><literal>???</literal></quote> + rather than the custom name assigned. Trying to understand why '???' is any better than 'extension' for a registered custom wait event of an unloaded extension? PS: Looked at other instances where '???' is being used for representing an unknown "thing". 2. Have an example of how a custom wait event is displayed in the example in the docs "Here is an example of how wait events can be viewed:". We can use the worker_spi wait event type there. 3. - so an <literal>LWLock</literal> wait event might be reported as - just <quote><literal>extension</literal></quote> rather than the - extension-assigned name. + <xref linkend="wait-event-lwlock-table"/>. In some cases, the name + assigned by an extension will not be available in all server processes + if the extension's library is not loaded; so a custom wait event might + be reported as just <quote><literal>???</literal></quote> Are we missing to explicitly say what wait event will be reported for an LWLock when the extension library is not loaded? 4. + Add-ins can define custom wait events under the wait event type I see a few instances of Add-ins/add-in in xfunc.sgml. Isn't it better to use the word extension given that glossary defines what an extension is https://www.postgresql.org/docs/current/glossary.html#GLOSSARY-EXTENSION? 5. +} WaitEventExtensionCounter; + +/* pointer to the shared memory */ +static WaitEventExtensionCounter *waitEventExtensionCounter; How about naming the structure variable as WaitEventExtensionCounterData and pointer as WaitEventExtensionCounter? This keeps all the static variable names consistent WaitEventExtensionNames, WaitEventExtensionNamesAllocated and WaitEventExtensionCounter. 6. + /* Check the wait event class. */ + Assert((wait_event_info & 0xFF000000) == PG_WAIT_EXTENSION); + + /* This should only be called for user-defined wait event. */ + Assert(eventId >= NUM_BUILTIN_WAIT_EVENT_EXTENSION); Maybe, we must turn the above asserts into ereport(ERROR) to protect against an extension sending in an unregistered wait_event_info? Especially, the first Assert((wait_event_info & 0xFF000000) == PG_WAIT_EXTENSION); checks that the passed in wait_event_info is previously returned by WaitEventExtensionNew. IMO, these assertions better fit for errors. 7. + * Extensions can define their own wait events in this categiry. First, Typo - s/categiry/category 8. + First, + * they should call WaitEventExtensionNew() to get one or more wait event + * IDs that are allocated from a shared counter. Can WaitEventExtensionNew() be WaitEventExtensionNew(int num_ids, int *result) to get the required number of wait event IDs in one call similar to RequestNamedLWLockTranche? Currently, an extension needs to call WaitEventExtensionNew() N number of times to get N wait event IDs. Maybe the existing WaitEventExtensionNew() is good, but just a thought. 9. # The expected result is a special pattern here with a newline coming from the # first query where the shared memory state is set. $result = $node->poll_query_until( 'postgres', qq[SELECT worker_spi_init(); SELECT wait_event FROM pg_stat_activity WHERE backend_type ~ 'worker_spi';], qq[ worker_spi_main]); This test doesn't have to be that complex with the result being a special pattern, SELECT worker_spi_init(); can just be within a separate safe_psql. 10. + wsstate = ShmemInitStruct("custom_wait_event", Name the shared memory just "worker_spi" to make it generic and extensible. Essentially, it is a woker_spi shared memory area part of it is for custom wait event id. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
В списке pgsql-hackers по дате отправления:
Следующее
От: Peter SmithДата:
Сообщение: Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication