Re: Draft for basic NUMA observability

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(at)anarazel(dot)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