Hi,
On Thu, Mar 13, 2025 at 02:15:14PM +0000, Bertrand Drouvot wrote:
> > > === 19
> > >
> >
> > Can you please take a look again on this
>
> Sure, will do.
> I'll have a look at v11 soon.
About 0001:
=== 1
git am produces:
.git/rebase-apply/patch:378: new blank line at EOF.
+
.git/rebase-apply/patch:411: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
=== 2
+ Returns true if a <acronym>NUMA</acronym> support is available.
What about "Returns true if the server has been compiled with NUMA support"?
=== 3
+Datum
+pg_numa_available(PG_FUNCTION_ARGS)
+{
+ if(pg_numa_init() == -1)
+ PG_RETURN_BOOL(false);
+ PG_RETURN_BOOL(true);
+}
What about PG_RETURN_BOOL(pg_numa_init() != -1)?
Also I wonder if pg_numa.c would not be a better place for it.
=== 4
+{ oid => '5102', descr => 'Is NUMA compilation available?',
+ proname => 'pg_numa_available', provolatile => 'v', prorettype => 'bool',
+ proargtypes => '', prosrc => 'pg_numa_available' },
+
Not sure that 5102 is a good choice.
src/include/catalog/unused_oids is telling me:
Best practice is to start with a random choice in the range 8000-9999.
Suggested random unused OID: 9685 (23 consecutive OID(s) available starting
here)
So maybe use 9685 instead?
=== 5
@@ -12477,3 +12481,4 @@
prosrc => 'gist_stratnum_common' },
]
+
garbage?
=== 6
Run pgindent as it looks like it's finding some things to do in
src/backend/storage/ipc/shmem.c
and src/port/pg_numa.c.
=== 7
+Size
+pg_numa_get_pagesize(void)
+{
+ Size os_page_size = sysconf(_SC_PAGESIZE);
+ if (huge_pages_status == HUGE_PAGES_ON)
+ GetHugePageSize(&os_page_size, NULL);
+ return os_page_size;
+}
I think that's more usual to:
Size
pg_numa_get_pagesize(void)
{
Size os_page_size = sysconf(_SC_PAGESIZE);
if (huge_pages_status == HUGE_PAGES_ON)
GetHugePageSize(&os_page_size, NULL);
return os_page_size;
}
I think that makes sense to check huge_pages_status as you did.
=== 8
+extern void numa_warn(int num, char *fmt,...) pg_attribute_printf(2, 3);
+extern void numa_error(char *where);
I wonder if that would make sense to add a comment mentioning
github.com/numactl/numactl/blob/master/libnuma.c here.
I still need to look at 0002 and 0003.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
