Re: BUG #14941: Vacuum crashes

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com>
Cc: 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: 2017-12-05 16:52:40
Message-ID: D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs pgsql-hackers

On 12/4/17, 8:52 AM, "Robert Haas" <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Dec 4, 2017 at 8:59 AM, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com> wrote:
>> sorry guys, yes I'm talking about a FULL VACUUM and not about Auto-Vacuum.
>> Thank you very match for your feedback.
>
> OK, but that's not the confusion. What you said is that it CRASHES,
> but the behavior you described is that it BLOCKS waiting for a lock.
> Blocking and crashing are not the same thing.

Provided that Lyes seems to have described wanting to avoid the blocking
behavior (and since I'm interested in adding this functionality anyway),
here are some patches that open up the VACOPT_NOWAIT option to users for
both VACUUM and ANALYZE commands.

---

0001_fix_unparenthesized_vacuum_grammar_v1.patch

One VacuumStmt grammar rule allows users to specify the VACOPT_ANALYZE
option by including an AnalyzeStmt at the end of the command. This
appears to have been added as part of the introduction of the ANALYZE
command (f905d65e). However, this means that users can call VACUUM
commands like

VACUUM VERBOSE ANALYZE VERBOSE pg_class;

Besides being disallowed according to the documentation, I think this
may give users the idea that there is a difference between the VERBOSE
options for VACUUM versus ANALYZE. In fact, they are the same option,
and specifying it twice is redundant.

This change fixes the unparenthesized VACUUM syntax to disallow the out-
of-order VACUUM options as described above. This is a prerequisite
change for opening up VACOPT_NOWAIT to users, as adding it to the
grammar as-is would cause statements like

VACUUM VERBOSE NOWAIT ANALYZE VERBOSE NOWAIT pg_class;

to be allowed. Since I am also looking to introduce a parenthesized
syntax for ANALYZE, this patch would prevent statements like

VACUUM VERBOSE ANALYZE (VERBOSE, NOWAIT) pg_class;

from being accepted as well.

---

0002_add_parenthesized_analyze_syntax_v1.patch

As a prerequisite to giving users access to the VACOPT_NOWAIT option, I
am introducing a parenthesized syntax for ANALYZE that is similar to
VACUUM's. Following VACUUM's example, any new options will be added to
this syntax, and the old style will become deprecated.

Adding a parenthesized syntax for ANALYZE is not strictly necessary for
providing the NOWAIT option, as NOWAIT is already a keyword in gram.y.
However, I thought it would be good to match VACUUM (since the command
structure is so similar), and this work would be required at some point
anyway if ANALYZE ever accepts options that we do not want to make
reserved keywords.

---

0003_add_nowait_vacuum_option_v1.patch

This change provides the existing VACOPT_NOWAIT option to users. This
option was previously only used by autovacuum in special cases, but it
seems like a potentially valuable option in other cases as well. For
example, perhaps a user wants to run a nightly VACUUM job that skips
tables that cannot be locked due to autovacuum (or something else)
already working on it.

I chose to use NOWAIT as the option name because it is already a keyword
for the LOCK command.

This patch follows the existing logging precedent added by 11d8d72c and
ab6eaee8: if a relation is explicitly specified in the command, a log
statement will be emitted if it is skipped. If the relation is not
specified (e.g. "VACUUM FULL;"), no such log statements will be emitted.

---

I'll be adding an entry to the next commitfest for these patches soon.

Nathan

Attachment Content-Type Size
0003_add_nowait_vacuum_option_v1.patch application/octet-stream 7.6 KB
0002_add_parenthesized_analyze_syntax_v1.patch application/octet-stream 3.6 KB
0001_fix_unparenthesized_vacuum_grammar_v1.patch application/octet-stream 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2017-12-05 17:03:48 Re: pg_dumpall -r -c try to drop user postgres
Previous Message Stephen Frost 2017-12-05 16:41:56 Re: [HACKERS] postgres_fdw super user checks

Browse pgsql-bugs by date

  From Date Subject
Next Message Jan Schulz 2017-12-05 17:48:42 Re: BUG #14948: cost overflow
Previous Message Tom Lane 2017-12-05 15:10:20 Re: BUG #14948: cost overflow