Re: parallel vacuum comments

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: "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>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: parallel vacuum comments
Date: 2021-12-17 05:30:31
Message-ID: CAD21AoD0r0rzHWXfPhHXD0R_eZ1_4GGfmj2cwhg++yDfwh-NSw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 16, 2021 at 4:27 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wed, Dec 15, 2021 4:03 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > On Tue, Dec 14, 2021 at 12:03 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > There is still pending
> > > work related to moving parallel vacuum code to a separate file and a
> > > few other pending comments that are still under discussion. We can
> > > take care of those in subsequent patches. Do, let me know if you or
> > > others think differently?
> >
> > I'm on the same page.
> >
> > I've attached an updated patch. The patch incorporated several changes from
> > the last version:
> >
> > * Rename parallel_vacuum_begin() to parallel_vacuum_init()
> > * Unify the terminology; use "index bulk-deletion" and "index cleanup"
> > instead of "index vacuum" and "index cleanup".
> > * Fix the comment of parallel_vacuum_init() pointed out by Andres
> > * Fix a typo that is left in commit 22bd3cbe0c (pointed out by Hou)
> >
> > Please review it.
>
> Thanks for updating the patch.
> Here are a few comments:

Thank you for the comments!

>
> 1)
> + case PARALLEL_INDVAC_STATUS_NEED_CLEANUP:
> + errcontext("while cleanup index \"%s\" of relation \"%s.%s\"",
>
> I noticed current code uses error msg "While cleaning up index xxx" which seems a little
> different from the patch's maybe we can use the previous one ?

Right. Fixed.

>
> 2)
> static inline Size max_items_to_alloc_size(int max_items);
>
> This old function declaration can be deleted.

Removed.

>
> 3)
> --- a/src/tools/pgindent/typedefs.list
> +++ b/src/tools/pgindent/typedefs.list
>
> I think we need to remove LVShared, LVSharedIndStats, LVDeadItems and
> LVParallelState from typedefs.list and add PVShared and PVIndStats to the file.

Fixed.

These comments are incorporated into the patch I just submitted[1].

Regards,

[1] https://www.postgresql.org/message-id/CAD21AoB66GqEjHttbRd0_fy9hnBPJp8kBCWnMq87mG6V_BODSA%40mail.gmail.com

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Yugo NAGATA 2021-12-17 05:41:26 Re: Allow DELETE to use ORDER BY and LIMIT/OFFSET
Previous Message Dinesh Chemuduru 2021-12-17 05:24:50 Re: [PROPOSAL] new diagnostic items for the dynamic sql