From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Peter Geoghegan <pg(at)bowt(dot)ie>, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: parallel vacuum comments |
Date: | 2021-12-22 12:52:01 |
Message-ID: | CAA4eK1LA7iz6USQqy0u=_df5C_NzmTeUAtC2DcMZDOYC4brOkQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Dec 22, 2021 at 5:39 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wed, Dec 22, 2021 11:36 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | houzj.fnst@fujitsu.com | 2021-12-22 13:11:38 | Delay the variable initialization in get_rel_sync_entry |
Previous Message | wangw.fnst@fujitsu.com | 2021-12-22 12:37:35 | RE: Failed transaction statistics to measure the logical replication progress |