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 <and...@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
