Re: Adding basic NUMA awareness - Mailing list pgsql-hackers

Hi Tomas,

Thank you very much for working on this problem and the entire line of 
patches prepared! I've tried to play with these patches a little and 
here are some my observations and suggestions.

In the current implementation we try to use all available NUMA nodes on 
the machine, however it's often useful to limit the database only to a 
set of specific nodes, so that other nodes can be used for other 
processes. In my testing I was trying to use one node out of four for 
the client program, so I'd liked to limit the database to the remaining 
nodes. I use a systemd service with AllowedMemoryNodes/AllowedCPUs to 
start the cluster, so the obvious choice for me was to use the 
'numa_get_membind' function instead of 'numa_num_configured_nodes' to 
get the list of usable nodes. However, it is much easier to work with 
logical nodes in the [0; n] range inside the PG code, so I've decided to 
add mapping between 'logical nodes' (0-n in PG) to a set of physical 
nodes actually returned by 'numa_get_membind'. We may need to map number 
in both directions, so two translation tables are allocated and filled 
at the first usage of 'pg_numa' functions. It also seems to be a good 
idea to isolate all 'libnuma' calls inside 'pg_numa.c', so to keep all 
'numa_...' calls in it and this also allows us to hide this mapping in 
static functions. Here is the patch, which I've used to test this idea: 
https://github.com/Lerm/postgres/commit/9ec625c2bf564f5432375ec1d7ad02e4b2559161. 
This idea probably could be extended by adding some view to expose this 
mapping to the user (at least for testing purposes) and allow to 
explicitly override this mapping with a GUC setting. With such GUC 
setting we would be able to control PG memory usage on NUMA nodes 
without the need for systemd resource control or numactl parameters.

Next, I've noticed some problems related to the size alignment for 
'numa_tonode_memory' call in 'pg_numa_move_to_node' function. The 
documentation for the 'numa_tonode_memory' says that 'The  size 
argument  will  be rounded up to a multiple of the system page size'. 
However this does not work well with huge pages as alignment is 
performed for the default kernel page size (i.e. 4K in most cases). If 
addr + size value (rounded to the default page size) does not cover the 
entire huge page, then such invocation seems to be processed incorrectly 
and allocation policy is not applied for next pages access in such 
segment. At least this was the behavior I've observed on Debian 12 / 
6.1.40 kernel (i.e. '/proc/<pid>/numa_maps' shows that the segment 
contains pages from wrong nodes).

There are two location at which we could face such situation in current 
patches. First is related to buffers partitions mapping. With current 
code we basically ensure that combined size of all partitions for a 
single node is aligned to (huge) page size (as size is bound to the 
number of descriptors on one page), but individual partition is not 
explicitly aligned to this size. So, we could get the situation in which 
single page is split between adjacent partitions (e.g. 32GB buffers 
split by 3 nodes). With current code we will try to map each partition 
independently, which will results in unaligned calls to 
'numa_tonode_memory', so resulting mapping will be incorrect. We could 
either try to choose size for individual partition to align it to the 
desired page size or map all the partitions for a single node using a 
single 'pg_numa_move_to_node' invocation. During testing I've used the 
second approach, so here is the change to implement such logic: 
https://github.com/Lerm/postgres/commit/ee8b3603afd6d89e67b755dadc8e4c25ffba88be.

The second location which could expose the same problem is related to 
the mapping of PGPROC arrays in 'pgproc_partition_init': here we need to 
align pointer to the end PGPROC partition. There seems to be also two 
additional problems with PGPROC partitioning: we need to account 
additional padding pages in 'PGProcShmemSize' (using the same logic as 
with fplocks) and we should not call 'MemSet(ptr, 0, ...)' prior to 
partitions mapping call (otherwise it will be mapped to current node). 
Here is a potential change, which tries to address these problems: 
https://github.com/Lerm/postgres/commit/eaf12776f59ff150735d0f187595fc8ce3f0a872.

There are also some potential problems with buffers distribution between 
nodes. I have a feeling that current logic in 
'buffer_partitions_prepare' does not work correctly if number of buffers 
is enough to cover just a single partition per node, but total number of 
nodes is below MIN_BUFFER_PARTITIONS (i.e. 2 or 3). In this case we will 
set 'numa_can_partition' to 'true', but will allocate 2 partitions per 
node (so, 4 or 6 partitions in total), while we can fill just 2 or 3 
partition and leaving remaining partitions empty. This should violate 
the last assert check, as last partition will get zero buffers in this 
case. Another issue is related to the usage of 1GB pages, as minimal 
size for buffers partitions is limited by the minimal number of buffer 
descriptors in a single page. For 2MB pages this gives 2097152 / 64 * 8K 
= 256M as minimal size for partition, but for 1GB page the minimal size 
is equal to 1GB / 64 * 8K  = 128GB. So, if we assume 4 as minimal number 
of partitions, then for 2MB pages we need just 1GB for shared_buffers to 
enable partitioning (which seems a perfectly fine minimal limit for most 
cases), but for 1GB pages we need to allocate at least 512GB to allow 
buffers partitioning. Certainly, 1GB pages are usually used on large 
machines with large number of buffers allocated, but still it may be 
useful to allow configurations with 32GB or 64GB buffer cache to use 
both 1GB pages and buffers partitioning at the same time. However, I 
don't see an easy way to achieve this with the current logic. We either 
need to allow usage of different page sizes here (i.e. 2MB for 
descriptors and 1GB for buffers) or combine both buffers and its 
descriptors in a single object (i.e. 'buffer chunk', which cover enough 
buffers and their descriptors to fit into one or several memory pages), 
effectively replacing both buffers and descriptors arrays with an array 
of such 'chunks'. The latter solution may also help with dynamic buffer 
cache resizing (as we may just add additional 'chunks' in this case) and 
also increase TLB-hits with 1GB page (as both descriptor and its buffer 
will be likely located in the same page). However, both these changes 
seems to be quite large.

I've tried also to run some benchmarks on my server: I've got some 
improvements in 'pgbench/tpcb-like'results - about 8%, but only with 
backends pinning to NUMA node (i.e. adjusting your previous pinning 
patch to 'debug_numa' GUC: 
https://github.com/Lerm/postgres/commit/5942a3e12c7c501aa9febb63972a039e7ce00c20). 
For 'select-only' scenario the gain is more substantial (about 15%), but 
these tests are tricky, as they are more sensitive to other server 
settings and specific functions layout in compiled code, so they need 
more checks.

Thank you again for sharing these patches!

Thanks,
Alexey