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>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>, Andres Freund <andres@anarazel.de>
Date: 2021-12-13T06:12:14Z
Lists: pgsql-hackers

Attachments

On Mon, Dec 13, 2021 at 2:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Dec 13, 2021 at 10:33 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Dec 10, 2021 at 9:08 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Thu, Dec 9, 2021 at 6:05 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Thu, Dec 9, 2021 at 7:44 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > Agreed with the above two points.
> > > >
> > > > I've attached updated patches that incorporated the above comments
> > > > too. Please review them.
> > > >
> > >
> > > I have made the following minor changes to the 0001 patch: (a) An
> > > assert was removed from dead_items_max_items() which I added back. (b)
> > > Removed an unnecessary semicolon from one of the statements in
> > > compute_parallel_vacuum_workers(). (c) Changed comments at a few
> > > places. (d) moved all parallel_vacuum_* related functions together.
> > > (e) ran pgindent and slightly modify the commit message.
> > >
> > > Let me know what you think of the attached?
> >
> > Thank you for updating the patch!
> >
> > The patch also moves some functions, e.g., update_index_statistics()
> > is moved without code changes. I agree to move functions for
> > consistency but that makes the review hard and the patch complicated.
> > I think it's better to do improving the parallel vacuum code and
> > moving functions in separate patches.
> >
>
> Okay, I thought it might be better to keep all parallel_vacuum_*
> related functions together but we can keep that in a separate patch
> Feel free to submit without those changes.

I've attached the patch. I've just moved some functions back but not
done other changes.

> In fact, if we go for your
> current 0002 then that might not be even required as we move all those
> functions to a new file.

Right. So it seems not necessary.

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.