Re: Should we increase the default vacuum_cost_limit?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Jeremy Schneider <schnjere(at)amazon(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Should we increase the default vacuum_cost_limit?
Date: 2019-03-09 21:04:39
Message-ID: 1798.1552165479@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> On Sat, Mar 9, 2019 at 7:58 PM Andrew Dunstan
> <andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
>> On 3/9/19 12:55 PM, Tom Lane wrote:
>>> The idea of converting vacuum_cost_delay into a float variable, while
>>> keeping its native unit as ms, seems probably more feasible from a
>>> compatibility standpoint. There are two sub-possibilities:
>>> ...
>>> 2. Add support for units for float variables, too. I don't think
>>> this'd be a huge amount of work, and we'd surely have other uses
>>> for it in the long run.
>>> ...
>>> I'm inclined to go look into #2. Anybody think this is a bad idea?

>> Sounds good to me, seems much more likely to be future-proof.

> Agreed.

I tried this, and it seems to work pretty well. The first of the two
attached patches just teaches guc.c to support units for float values,
incidentally allowing "us" as an input unit for time-based GUCs.
The second converts [autovacuum_]cost_delay to float GUCs, and changes
the default value for autovacuum_cost_delay from 20ms to 2ms.
We'd want to revert the previous patch that changed the default value
of vacuum_cost_limit, else this means a 100x not 10x change in the
default autovac speed ... but I've not included that in this patch.

Some notes:

1. I hadn't quite absorbed until doing this that we'd need a catversion
bump because of format change in StdRdOptions. Since this isn't proposed
for back-patching, that doesn't seem problematic.

2. It's always bugged me that we don't allow fractional unit
specifications, say "0.1GB", even for GUCs that are integers underneath.
That would be a simple additional change on top of this, but I didn't
do it here.

3. I noticed that parse_real doesn't reject infinity or NaN values
for float GUCs. This seems like a bug, maybe even a back-patchable one;
I doubt the planner will react sanely to SET seq_page_cost TO 'NaN'
for instance. I didn't do anything about that here either.

4. I've not done anything here about increasing the max value of
[autovacuum_]vacuum_cost_limit. I have no objection to kicking that
up 10x or so if somebody wants to do the work, but I'm not sure it's
very useful given this patch.

Comments?

regards, tom lane

Attachment Content-Type Size
0001-allow-units-for-float-GUCs.patch text/x-diff 17.4 KB
0002-make-cost-delay-GUCs-float.patch text/x-diff 14.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-03-09 21:06:33 Re: Should we increase the default vacuum_cost_limit?
Previous Message Karl O. Pinc 2019-03-09 20:53:52 Re: Patch to document base64 encoding