Re: parallel vacuum comments
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>,
Peter Geoghegan <pg@bowt.ie>, Andres Freund <andres@anarazel.de>
Date: 2021-12-22T12:52:01Z
Lists: pgsql-hackers
On Wed, Dec 22, 2021 at 5:39 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Wed, Dec 22, 2021 11:36 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
>
> The patch looks mostly good to me.
> I only have few comments.
>
> 1)
> +/*
> + * Do parallel index bulk-deletion with parallel workers.
> + */
> +void
> +parallel_vacuum_bulkdel_all_indexes(ParallelVacuumState *pvs, long num_table_tuples)
> +{
> + Assert(!IsParallelWorker());
> +
>
> Would it be better to also put Assert(pvs != NULL) here ? Because we removed
> the Assert(ParallelVacuumIsActive(vacrel)) check in the old function.
>
I am not sure if that is helpful or not because there is only one
caller of it which checks pvs before calling this function.
>
> 2)
> +#include "utils/rel.h"
> +#include "utils/lsyscache.h"
> +#include "utils/memutils.h"
>
> It might be better to keep the header file in alphabetical order.
> :
> +#include "utils/lsyscache.h"
> +#include "utils/memutils.h"
> +#include "utils/rel.h"
>
Right, I'll take care of this as I am already making some other edits
in the patch.
--
With Regards,
Amit Kapila.
Commits
-
Move parallel vacuum code to vacuumparallel.c.
- 8e1fae193864 15.0 landed
-
Move index vacuum routines to vacuum.c.
- cc8b25712b5e 15.0 landed
-
Improve parallel vacuum implementation.
- 22bd3cbe0c28 15.0 landed
-
Fix parallel amvacuumcleanup safety bug.
- c59278a1aa5e 15.0 landed
- f6162c020c88 14.1 landed
-
Simplify state managed by VACUUM.
- b4af70cb2103 14.0 cited