Re: Increase of maintenance_work_mem limit in 64-bit Windows

David Rowley писал(а) 2024-09-23 04:28:
> On Fri, 20 Sept 2024 at 01:55, Пополитов Владлен
> <v.popolitov@postgrespro.ru> wrote:
>> Currently PostgreSQL built on 64-bit Windows has 2Gb limit for
>> GUC variables due to sizeof(long)==4 used by Windows compilers.
>> Technically 64-bit addressing for maintenance_work_mem is possible,
>> but code base historically uses variables and constants of type 
>> "long",
>> when process maintenance_work_mem value.
> 
> I agree. Ideally, we shouldn't use longs for anything ever. We should
> likely adopt trying to remove the usages of them when possible.
> 
> I'd like to suggest you go about this patch slightly differently with
> the end goal of removing the limitation from maintenance_work_mem,
> work_mem, autovacuum_work_mem and logical_decoding_work_mem.
> 
> Patch 0001: Add a macro named something like WORK_MEM_KB_TO_BYTES()
> and adjust all places where we do <work_mem_var> * 1024L to use this
> new macro. Make the macro do the * 1024L as is done today so that this
> patch is a simple refactor.
> Patch 0002: Convert all places that use long and use Size instead.
> Adjust WORK_MEM_KB_TO_BYTES to use a Size type rather than 1024L.
> 
> It might be wise to break 0002 down into individual GUCs as the patch
> might become large.
> 
> I suspect we might have quite a large number of subtle bugs in our
> code today due to using longs. 7340d9362 is an example of one that was
> fixed recently.
> 
> David

Hi David,
Thank you for proposal, I looked at the patch and source code from this
point of view. In this approach we need to change all <work_mem_var>.
I counted the appearences of these vars in the code:
maintenance_work_mem appears 63 times in 20 files
work_mem appears 113 times in 48 files
logical_decoding_work_mem appears 10 times in 2 files
max_stack_depth appears 11 times in 3 files
wal_keep_size_mb appears 5 times in 3 files
min_wal_size_mb appears 5 times in 2 files
max_wal_size_mb appears 10 times in 2 files
wal_skip_threshold appears 5 times in 2 files
max_slot_wal_keep_size_mb appears 6 times in 3 files
wal_sender_timeout appears 23 times in 3 files
autovacuum_work_mem appears 11 times in 4 files
gin_pending_list_limit appears 8 times in 5 files
pendingListCleanupSize appears 2 times in 2 files
GinGetPendingListCleanupSize appears 2 times in 2 files

maintenance_work_mem appears 63 times and had only 4 cases, where "long"
is used (I fix it in patch). I also found, that this patch also fixed
autovacuum_work_mem , that has only 1 case - the same place in code as
maintenance_work_mem.

Now <work_mem_vars> in the code are processed based on the context: they 
are
assigned to Size, uint64, int64, double, long, int variables (last 2 
cases
need to fix) or multiplied by (uint64)1024, (Size)1024, 1024L (last case
needs to fix). Also signed value is used for max_stack_depth (-1 used as
error value). I am not sure, that we can solve all this cases by one
macro WORK_MEM_KB_TO_BYTES(). The code needs case by case check.

If I check the rest of the variables, the patch does not need
MAX_SIZE_T_KILOBYTES constant (I introduced it for variables, that are
already checked and fixed), it will contain only fixes in the types of
the variables and the constants.
It requires a lot of time to check all appearances and neighbour
code, but final patch will not be large, I do not expect a lot of
"long" in the rest of the code (only 4 case out of 63 needed to fix
for maintenance_work_mem).
What do you think about this approach?

-- 
Best regards,

Vladlen Popolitov.