Re: [HACKERS] Block level parallel vacuum

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, David Steele <david(at)pgmasters(dot)net>, Claudio Freire <klaussfreire(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Block level parallel vacuum
Date: 2019-03-22 05:02:36
Message-ID: CAD21AoD7rqZPPyV7z4bku8Mn8AE2_kRdW1hTO4Lrsp+vn_U1kQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 22, 2019 at 4:53 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Mar 19, 2019 at 7:26 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > In parsing vacuum command, since only PARALLEL option can have an
> > argument I've added the check in ExecVacuum to erroring out when other
> > options have an argument. But it might be good to make other vacuum
> > options (perhaps except for DISABLE_PAGE_SKIPPING option) accept an
> > argument just like EXPLAIN command.
>
> I think all of the existing options, including DISABLE_PAGE_SKIPPING,
> should permit an argument that is passed to defGetBoolean().
>

Agreed. The attached 0001 patch changes so.

On Thu, Mar 21, 2019 at 8:05 PM Sergei Kornilov <sk(at)zsrv(dot)org> wrote:
>
> Hello
>

Thank you for reviewing the patch!

> > * in_parallel is true if we're performing parallel lazy vacuum. Since any
> > * updates are not allowed during parallel mode we don't update statistics
> > * but set the index bulk-deletion result to *stats. Otherwise we update it
> > * and set NULL.
>
> lazy_cleanup_index has in_parallel argument only for this purpose, but caller still should check in_parallel after lazy_cleanup_index call and do something else with stats for parallel execution.
> Would be better always return stats and update statistics in caller? It's possible to update all index stats in lazy_vacuum_all_indexes for example? This routine is always parallel leader and has comment /* Do post-vacuum cleanup and statistics update for each index */ on for_cleanup=true call.

Agreed. I've changed the patch so that we update index statistics in
lazy_vacuum_all_indexes().

>
> I think we need note in documentation that parallel leader is not counted in PARALLEL N option, so with PARALLEL 2 option we want use 3 processes. Or even change behavior? Default with PARALLEL 1 - only current backend in single process is running, PARALLEL 2 - leader + one parallel worker, two processes works in parallel.
>

Hmm, the documentation says "Perform vacuum index and cleanup index
phases of VACUUM in parallel using N background workers". Doesn't it
already explain that?

Attached the updated version patch. 0001 patch allows all existing
vacuum options an boolean argument. 0002 patch introduces parallel
lazy vacuum. 0003 patch adds -P (--parallel) option to vacuumdb
command.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachment Content-Type Size
v19-0001-All-VACUUM-command-options-allow-an-argument.patch application/octet-stream 5.2 KB
v19-0003-Add-paralell-P-option-to-vacuumdb-command.patch application/octet-stream 6.0 KB
v19-0002-Add-parallel-option-to-VACUUM-command.patch application/octet-stream 54.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-03-22 05:09:57 Re: BUG #15668: Server crash in transformPartitionRangeBounds
Previous Message Alvaro Herrera 2019-03-22 04:33:59 Re: Transaction commits VS Transaction commits (with parallel) VS query mean time