Re: Increase of maintenance_work_mem limit in 64-bit Windows

David Rowley писал(а) 2024-09-23 15:35:
> On Mon, 23 Sept 2024 at 21:01, Vladlen Popolitov
> <v(dot)popolitov(at)postgrespro(dot)ru> wrote:
>> 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
>
> Why do you think all of these appearances matter? I imagined all you
> care about are when the values are multiplied by 1024.
Common pattern in code - assign <work_mem_var> to local variable and
send
local variable as parameter to function, then to nested function, and
somewhere deep multiply function parameter by 1024. It is why I needed
to
check all appearances, most of them are correct.
>> 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?
>
> I don't think you can do maintenance_work_mem without fixing work_mem
> too. I don't think the hacks you've put into RI_Initial_Check() to
> ensure you don't try to set work_mem beyond its allowed range are very
> good. It effectively means that maintenance_work_mem does not do what
> it's meant to for the initial validation of referential integrity
> checks. If you're not planning on fixing work_mem too, would you just
> propose to leave those hacks in there forever?
I agree, it is better to fix all them together. I also do not like this
hack, it will be removed from the patch, if I check and change
all <work_mem_vars> at once.
I think, it will take about 1 week to fix and test all changes. I will
estimate the total volume of the changes and think, how to group them
in the patch ( I hope, it will be only one patch)

--
Best regards,

Vladlen Popolitov.