David Rowley писал(а) 2024-09-23 15:35:
On Mon, 23 Sept 2024 at 21:01, Vladlen Popolitov
<v.popoli...@postgrespro.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.
