Re: Draft for basic NUMA observability
От | Bertrand Drouvot |
---|---|
Тема | Re: Draft for basic NUMA observability |
Дата | |
Msg-id | Z+qtwfrhD+58E9ub@ip-10-97-1-34.eu-west-3.compute.internal обсуждение исходный текст |
Ответ на | Re: Draft for basic NUMA observability (Jakub Wartak <jakub.wartak@enterprisedb.com>) |
Ответы |
Re: Draft for basic NUMA observability
|
Список | pgsql-hackers |
Hi, On Mon, Mar 31, 2025 at 11:27:50AM +0200, Jakub Wartak wrote: > On Thu, Mar 27, 2025 at 2:40 PM Andres Freund <andres@anarazel.de> wrote: > > > > > +Size > > > +pg_numa_get_pagesize(void) > [..] > > > > Should this have a comment or an assertion that it can only be used after > > shared memory startup? Because before that huge_pages_status won't be > > meaningful? > > Added both. Thanks for the updated version! + Assert(IsUnderPostmaster); I wonder if that would make more sense to add an assertion on huge_pages_status and HUGE_PAGES_UNKNOWN instead (more or less as it is done in CreateSharedMemoryAndSemaphores()). === About v17-0002-This-extracts-code-from-contrib-pg_buffercache-s.patch Once applied I can see mention to pg_buffercache_numa_pages() while it only comes in v17-0003-Extend-pg_buffercache-with-new-view-pg_buffercac.patch. I think pg_buffercache_numa_pages() should not be mentioned before it's actually implemented. === 1 + bufRecord->isvalid == false) { int i; - funcctx = SRF_FIRSTCALL_INIT(); - - /* Switch context when allocating stuff to be used in later calls */ - oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); - - /* Create a user function context for cross-call persistence */ - fctx = (BufferCachePagesContext *) palloc(sizeof(BufferCachePagesContext)); + for (i = 1; i <= 9; i++) + nulls[i] = true; "i <= 9" will be correct only once v17-0003 is applied (when NUM_BUFFERCACHE_PAGES_ELEM is increased to 10). In v17-0002 that should be i < 9 (even better i < NUM_BUFFERCACHE_PAGES_ELEM). That could also make sense to remove the loop and use memset() that way: " memset(&nulls[1], true, (NUM_BUFFERCACHE_PAGES_ELEM - 1) * sizeof(bool)); " instead. It's done that way in some other places (hbafuncs.c for example). === 2 - if (expected_tupledesc->natts == NUM_BUFFERCACHE_PAGES_ELEM) - TupleDescInitEntry(tupledesc, (AttrNumber) 9, "pinning_backends", - INT4OID, -1, 0); + if (expected_tupledesc->natts >= NUM_BUFFERCACHE_PAGES_ELEM - 1) + TupleDescInitEntry(tupledesc, (AttrNumber) 9, "pinning_backends", + INT4OID, -1, 0); I think we should not change the "expected_tupledesc->natts" check here until v17-0003 is applied. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
В списке pgsql-hackers по дате отправления: