Re: BUG #14941: Vacuum crashes

From: Andres Freund <andres(at)anarazel(dot)de>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #14941: Vacuum crashes
Date: 2018-03-04 00:12:52
Message-ID: 20180304001252.5sxn3glccnu5qxjm@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs pgsql-hackers

On 2018-01-11 08:14:42 +0900, Michael Paquier wrote:
> On Wed, Jan 10, 2018 at 05:26:43PM +0000, Bossart, Nathan wrote:
> > Perhaps any logging changes for VACOPT_NOWAIT could be handled in a
> > separate thread. For now, I've updated 0003 to remove the logging
> > changes.
>
> Thanks. I am marking those as ready for committer, you are providing the
> set patch patch which offer the most consistent experience.

I was working on committing 0002 and 0003, when I noticed that the
second patch doesn't actually fully works. NOWAIT does what it says on
the tin iff the table is locked with a lower lock level than access
exclusive. But if AEL is used, the command is blocked in

static List *
expand_vacuum_rel(VacuumRelation *vrel)
...
/*
* We transiently take AccessShareLock to protect the syscache lookup
* below, as well as find_all_inheritors's expectation that the caller
* holds some lock on the starting relation.
*/
relid = RangeVarGetRelid(vrel->relation, AccessShareLock, false);

ISTM has been added after the patches initially were proposed. See
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=19de0ab23ccba12567c18640f00b49f01471018d

I'm a bit disappointed that the tests didn't catch this, they're clearly
not fully there. They definitely should test the AEL case, as
demonstrated here.

Independent of that, I'm also concerned that NOWAIT isn't implemented
consistently with other commands. Aren't we erroring out in other uses
of NOWAIT? ISTM a more appropriate keyword would have been SKIP
LOCKED. I think the behaviour makes sense, but I'd rename the internal
flag and the grammar to use SKIP LOCKED.

Lightly edited patches attached. Please preserve commit messages while
fixing these issues.

Greetings,

Andres Freund

Attachment Content-Type Size
v5-0001-Add-parenthesized-options-syntax-for-ANALYZE.patch text/x-diff 4.7 KB
v5-0002-Add-NOWAIT-option-to-VACUUM-and-ANALYZE.patch text/x-diff 9.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2018-03-04 08:59:07 BUG #15097: pgadmin3 tabs not properly themed with wxGTK3
Previous Message Andrew Gierth 2018-03-03 19:55:41 Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identity column

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2018-03-04 00:14:06 Re: WIP: BRIN multi-range indexes
Previous Message Alexander Korotkov 2018-03-03 23:59:19 Re: [HACKERS] GUC for cleanup indexes threshold.