Thread: Bug in VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL

Bug in VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL

From
Simon Riggs
Date:
In vacuumlazy.c, VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL is described as
being in ms on line 85, yet it is used on line 1759 in a call to
pg_usleep, so is treated as microseconds rather than milliseconds.

As a result, the timeout during lazy_truncate_heap() is actually only
5ms long, not 5s long.

So this looks to me like a bug, patch attached, for back-patching to 9.1
vacuum_lock_wait_ms.v1,patch

Objections?



Note that VACUUM_TRUNCATE_LOCK_CHECK_INTERVAL is described as being in
ms and is actually in ms, so no change there.


I'm also wondering why we don't use lock_timeout when the user sets it?
Not a bug, but patch attached anyway.
vacuum_truncate_use_lock_timeout.v1.patch

--
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: Bug in VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL

From
Robert Haas
Date:
On Tue, Sep 6, 2016 at 4:00 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> I'm also wondering why we don't use lock_timeout when the user sets it?
> Not a bug, but patch attached anyway.
> vacuum_truncate_use_lock_timeout.v1.patch

This part seems fairly random.  I don't think it makes sense to assume
that the timeout after which the user wants a lock acquisition request
to error out is the same time that they want as the interval between
retries.  Those things seem fairly thoroughly unconnected, and this
change could fairly easily cause truncation problems for people who
have the lock timeout set to a relatively long time (e.g. 10 minutes).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Bug in VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL

From
Simon Riggs
Date:
On 6 September 2016 at 11:30, Simon Riggs <simon@2ndquadrant.com> wrote:
> In vacuumlazy.c, VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL is described as
> being in ms on line 85, yet it is used on line 1759 in a call to
> pg_usleep, so is treated as microseconds rather than milliseconds.
>
> As a result, the timeout during lazy_truncate_heap() is actually only
> 5ms long, not 5s long.
>
> So this looks to me like a bug, patch attached, for back-patching to 9.1
> vacuum_lock_wait_ms.v1,patch
>
> Objections?

I'm inclined to backpatch this for 9.6 only, unless I hear more.

For earlier releases this will suddenly make VACUUMs take 5 secs where
they used to take 50ms, which could be a big surprise for some people
running database wide VACUUMs.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services