Re: autovacuum_work_mem

On Sun, Oct 20, 2013 at 2:22 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> There has recently been considerable discussion around auto-tuning.
> Throughout the course of this discussion, I raised the idea of
> creating a new GUC to separately control autovacuum's usage of
> maintenance_work_mem [1], explaining the rationale in some detail [2].
> At the time Magnus seemed to think this a good idea [3].
>
> Attached simple patch adds a new GUC, autovacuum_work_mem, which when
> set to a value other than -1 (the default) is preferred by autovacuum
> over maintenance_work_mem. All other usage of VACUUM is unaffected.
>
> I won't repeat the rationale for the patch here. Appropriate
> documentation changes are included. I don't think it's a problem that
> autovacuum_work_mem is kind of similar to vacuum_mem in name.
> maintenance_work_mem was last spelt vacuum_mem about 10 years ago.
> Enough time has passed that I think it very unlikely that someone
> might conflate the two.

My rationale for liking the idea is also in those threads :)

And I definitely don't think there is any problem whatsoever in having
a name that's close to something we had 10 years ago. I think it would
be hard enough to make a case for not reusing the *same* name that
long after, but here it's actually different.

> I have decided to have the default value of -1 carry, and not have it
> automatically take the same value as maintenance_work_mem, because
> doing so could create the impression that it needs to be set
> explicitly, which of course it does not - this is not the same
> situation as exists for wal_buffers. We just check if its set when
> going to VACUUM, if VACUUM is requested from a worker process.

It is the same that exists for autovacuum_vacuum_cost_limit and
autovacuum_vacuum_cost_delay, so that makes perfect sense.

> It seemed neater to me to create a new flag, so that in principle any
> vacuum() code path can request autovacuum_work_mem, rather than having
> lazyvacuum.c code call IsAutoVacuumWorkerProcess() for the same
> purpose. To date, that's only been done within vacuumlazy.c for things
> like logging.

Hmm. I'm not entirely sure I agree that that makes it neater :)

We could also look at autovacuum_vacuum_cost_limit etc above, but
those just override what the non-autovac parameters do. But since the
parameter is called maintenance_work_mem in that case, I think that
would make it harder to read.

But I'd suggest just a:
int vac_work_mem = (IsAutoVacuumWorkerProcess() && autovacuum_work_mem
!= -1) ? autovacuum_work_mem : maintenance_work_mem;

and not sending around a boolean flag through a bunch of places when
it really means just the same thing,

Another option would be to just have vac_work_mem actually being the
global,and have it set by the maintenance_work_mem parameter guc, and
then overwritten by the vacuum work mem parameter guc in autovacuum
worker startup. Then you wouldn't put either of those two in the
codepath.

Also, isn't this quite confusing:
+ # Note: autovacuum only prefers autovacuum_work_mem over maintenance_work_mem
+ #autovacuum_work_mem = -1 # min 1MB, or -1 to disable

Where does the "only" come from? And we don't really use the term
"prefers over" anywhere else there. And -1 doesn't actually disable
it. I suggest following the pattern of the other parameters that work
the same way, which would then just be:

#autovacuum_work_mem = -1 # amount of memory for each autovacuum
worker. -1 means use maintenance_work_mem

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/