From: Justin Pryzby <pryzby@telsasoft.com>
To: Mahendra Singh Thalor <mahi6run@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: tushar <tushar.ahuja@enterprisedb.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
Date: Wed, 8 Apr 2020 13:38:54 -0500
Message-ID: <20200408183854.GG2228@telsasoft.com> (raw)
In-Reply-To: <CAKYtNAogYT+ZNN9Q3eCpdWHVsYR5R-o5p0NtOZHLOkYP-4dxsA@mail.gmail.com>
References: <58c8d171-e665-6fa3-a9d3-d9423b694dae@enterprisedb.com>
<CA+TgmoY6JBix-20yS=EicJVHNAH44QnYGQhyR_c15dZi02KF-w@mail.gmail.com>
<CAKYtNAqzJ8rTh9Mb+2XdsH0dyZb2MS_XH-w8vz3i347etnaFSQ@mail.gmail.com>
<CA+TgmoYkFYRnmTLgPLPm664UdQFBcJTcDZXzBP6dG0zTniYEWQ@mail.gmail.com>
<20200408164154.GD2228@telsasoft.com>
<CAKYtNAogYT+ZNN9Q3eCpdWHVsYR5R-o5p0NtOZHLOkYP-4dxsA@mail.gmail.com>
On Thu, Apr 09, 2020 at 12:06:04AM +0530, Mahendra Singh Thalor wrote:
> On Wed, 8 Apr 2020 at 22:11, Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Wed, Apr 08, 2020 at 11:57:08AM -0400, Robert Haas wrote:
> > > On Wed, Apr 8, 2020 at 10:25 AM Mahendra Singh Thalor
> > > <mahi6run@gmail.com> wrote:
> > > > I think, Tushar point is that either we should allow both
> > > > vacuum(parallel 0, full 1) and vacuum(parallel 1, full 0) or in the
> > > > both cases, we should through error.
> > >
> > > Oh, yeah, good point. Somebody must not've been careful enough with
> > > the options-checking code.
> >
> > Actually I think someone was too careful.
> >
> > From 9256cdb0a77fb33194727e265a346407921055ef Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <pryzbyj@telsasoft.com>
> > Date: Wed, 8 Apr 2020 11:38:36 -0500
> > Subject: [PATCH v1] parallel vacuum: options check to use same test as in
> > vacuumlazy.c
> >
> > ---
> > src/backend/commands/vacuum.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
> > index 351d5215a9..660c854d49 100644
> > --- a/src/backend/commands/vacuum.c
> > +++ b/src/backend/commands/vacuum.c
> > @@ -104,7 +104,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
> > bool freeze = false;
> > bool full = false;
> > bool disable_page_skipping = false;
> > - bool parallel_option = false;
> > ListCell *lc;
> >
> > /* Set default value */
> > @@ -145,7 +144,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
> > params.truncate = get_vacopt_ternary_value(opt);
> > else if (strcmp(opt->defname, "parallel") == 0)
> > {
> > - parallel_option = true;
> > if (opt->arg == NULL)
> > {
> > ereport(ERROR,
> > @@ -199,7 +197,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
> > !(params.options & (VACOPT_FULL | VACOPT_FREEZE)));
> > Assert(!(params.options & VACOPT_SKIPTOAST));
> >
> > - if ((params.options & VACOPT_FULL) && parallel_option)
> > + if ((params.options & VACOPT_FULL) && params.nworkers > 0)
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > errmsg("cannot specify both FULL and PARALLEL options")));
> > --
> > 2.17.0
> >
>
> Thanks Justin for the patch.
>
> Patch looks fine to me and it is fixing the issue. After applying this
> patch, vacuum will work as:
>
> 1) vacuum (parallel 1, full 0);
> -- vacuuming will be done with 1 parallel worker.
> 2) vacuum (parallel 0, full 1);
> -- full vacuuming will be done.
> 3) vacuum (parallel 1, full 1);
> -- will give error :ERROR: cannot specify both FULL and PARALLEL options
>
> 3rd example is telling that we can't give both FULL and PARALLEL
> options but in 1st and 2nd, we are giving both FULL and PARALLEL
> options and we are not giving any error. I think, irrespective of
> value of both FULL and PARALLEL options, we should give error in all
> the above mentioned three cases.
I think the behavior is correct, but the error message could be improved, like:
|ERROR: cannot specify FULL with PARALLEL jobs
or similar.
--
Justin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, mahi6run@gmail.com, robertmhaas@gmail.com, tushar.ahuja@enterprisedb.com
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
In-Reply-To: <20200408183854.GG2228@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox