From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
cc: David Rowley <david.rowley@2ndquadrant.com>,
        Jeff Janes <jeff.janes@gmail.com>,
        Jeremy Schneider <schnjere@amazon.com>,
        Joe Conway <mail@joeconway.com>, Peter Geoghegan <pg@bowt.ie>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Should we increase the default vacuum_cost_limit?
In-reply-to: <13d143dd-0a44-0c16-218c-bc924b5922f0@2ndQuadrant.com>
References: 
 <CAKJS1f_YbXC2qTMPyCbmsPiKvZYwpuQNQMohiRXLj1r=8_rYvw@mail.gmail.com>
 <CAH2-Wz=WQ9ju6sjPF7RcW0g6NPXAenZeC5qVme6uj+eMRw5q2g@mail.gmail.com>
 <241b80ab-7af0-c606-7b2c-b0a5d6be696a@joeconway.com>
 <CAKJS1f_5xofbg0K16MkhQPeCXV6oGCuD2SVcV76OW7ydFDHDkw@mail.gmail.com>
 <df548503-d261-d8c4-97d0-61be6617df41@2ndQuadrant.com>
 <4a068b09-5a6f-e975-07c9-2333fcef0a8a@amazon.com>
 <30411a5b-ef11-71ef-e9ee-4d26ef3fd0c0@2ndQuadrant.com>
 <CAMkU=1zM7sK9Xo2jjwCLj-t+GHR5nQJnPyFUp+puBiFpOQ7aBA@mail.gmail.com>
 <21034.1552068630@sss.pgh.pa.us>
 <CAKJS1f9wbS+SzEDUXyMuLCsNgwMH=1Ztj3QE3WxuKdJtbqrOEA@mail.gmail.com>
 <13d143dd-0a44-0c16-218c-bc924b5922f0@2ndQuadrant.com>
Comments: In-reply-to Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
	message dated "Fri, 08 Mar 2019 20:29:14 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <26655.1552096451.1@sss.pgh.pa.us>
Date: Fri, 08 Mar 2019 20:54:11 -0500
Message-ID: <26656.1552096451@sss.pgh.pa.us>
Precedence: bulk

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> Increase it to what?

Jeff's opinion that it could be INT_MAX without causing trouble is
a bit over-optimistic, see vacuum_delay_point():

    if (VacuumCostActive && !InterruptPending &&
        VacuumCostBalance >= VacuumCostLimit)
    {
        int            msec;

        msec = VacuumCostDelay * VacuumCostBalance / VacuumCostLimit;

In the first place, if VacuumCostLimit is too close to INT_MAX,
it'd be possible for VacuumCostBalance (also an int) to overflow
between visits to vacuum_delay_point, wrapping around to negative
and thus missing the need to nap altogether.

In the second place, since large VacuumCostLimit implies large
VacuumCostBalance when we do get through this if-check, there's
a hazard of integer overflow in the VacuumCostDelay * VacuumCostBalance
multiplication.  The final value of the msec calculation should be
easily within integer range, since VacuumCostDelay is constrained to
not be very large, but that's no help if we have intermediate overflow.

Possibly we could forestall both of those problems by changing
VacuumCostBalance to double, but that would make the cost
bookkeeping noticeably more expensive than it used to be.
I think it'd be better to keep VacuumCostBalance as int,
which would then mean we'd better limit VacuumCostLimit to no
more than say INT_MAX/2 --- call it 1 billion for the sake of
a round number.

That'd still leave us at risk of an integer overflow in the
msec-to-sleep calculation, but that calculation could be changed
to double at little price, since once we get here we're going
to sleep awhile anyway.

BTW, don't forget autovacuum_cost_limit should have the same maximum.

			regards, tom lane