On Sat, 1 Apr 2023 at 12:57, Melanie Plageman <melanieplageman@gmail.com> wrote:
> I've attached v7 with that commit dropped and with support for parallel
> vacuum workers to use the same number of buffers in their own Buffer
> Access Strategy ring as the main vacuum phase did. I also updated the
> docs to indicate that vacuum_buffer_usage_limit is per backend (not per
> instance of VACUUM).
(was just replying about v6-0002 when this came in. Replying here instead)
For v7-0001, can we just get rid of both of those static globals? I'm
gobsmacked by the existing "A few variables that don't seem worth
passing around as parameters" comment. Not wanting to pass parameters
around is a horrible excuse for adding global variables, even static
ones.
Attached is what I propose in .diff form so that the CFbot can run on
your v7 patches without picking this up.
I considered if we could switch memory contexts before calling
expand_vacuum_rel() and get_all_vacuum_rels(), but I see, at least in
the case of expand_vacuum_rel() that we'd probably want to list_free()
the output of find_all_inheritors() to save that from leaking into the
vac_context. It seems safe just to switch into the vac_context only
when we really want to keep that memory around. (I do think switching
in each iteration of the foreach(part_lc, part_oids) loop is
excessive, however. Just not enough for me to want to change it)
David