Re: parallel vacuum comments

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.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-24T03:15:06Z
Lists: pgsql-hackers
On Fri, Dec 24, 2021 at 11:59 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Dec 23, 2021 at 10:56 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Dec 22, 2021 at 10:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Dec 22, 2021 at 6:22 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Wed, Dec 22, 2021 at 5:39 PM houzj.fnst@fujitsu.com
> > > > <houzj.fnst@fujitsu.com> wrote:
> > > > >
> > > > >
> > > > > 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.
> > > >
> > >
> > > Fixed this and made a few other changes in the patch that includes (a)
> > > passed down the num_index_scans information in parallel APIs based on
> > > which it can make the decision whether to reinitialize DSM or consider
> > > conditional parallel vacuum clean up; (b) get rid of first-time
> > > variable in ParallelVacuumState as that is not required if we have
> > > num_index_scans information; (c) there seems to be quite a few
> > > unnecessary includes in vacuumparallel.c which I have removed; (d)
> > > unnecessary error callback info was being set in ParallelVacuumState
> > > in leader backend; (e) changed/added comments at quite a few places.
> > >
> > > Can you please once verify the changes in the attached?
> >
> > Thank you for updating the patch!
> >
> > I agreed with these changes and it looks good to me.
> >
>
> Pushed.

Thank you for committing the patch!

> As per my knowledge, we have addressed the improvements
> raised/discussed in this thread.

Yeah, I think so too.

Regards,

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



Commits

  1. Move parallel vacuum code to vacuumparallel.c.

  2. Move index vacuum routines to vacuum.c.

  3. Improve parallel vacuum implementation.

  4. Fix parallel amvacuumcleanup safety bug.

  5. Simplify state managed by VACUUM.