Re: parallel vacuum comments

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@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-11-24T08:54:09Z
Lists: pgsql-hackers
On Wed, Nov 24, 2021 at 12:16 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Nov 24, 2021 at 1:34 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Nov 24, 2021 at 7:43 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > On Mon, Nov 22, 2021 at 1:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Fri, Nov 19, 2021 at 7:55 AM houzj.fnst@fujitsu.com
> > > > <houzj.fnst@fujitsu.com> wrote:
> > > >
> > > > >
> > > > > 4)
> > > > >
> > > > > Just a personal suggestion for the parallel related function name. Since Andres
> > > > > wanted a uniform naming pattern. Mabe we can rename the following functions:
> > > > >
> > > > > end|begin_parallel_vacuum => parallel_vacuum_end|begin
> > > > > perform_parallel_index_bulkdel|cleanup => parallel_vacuum_index_bulkdel|cleanup
> > > > >
> > > > > So that all the parallel related functions' name is like parallel_vacuum_xxx.
> > > > >
> > > >
> > > > BTW, do we really need functions
> > > > perform_parallel_index_bulkdel|cleanup? Both do some minimal
> > > > assignments and then call parallel_vacuum_all_indexes() and there is
> > > > just one caller of each. Isn't it better to just do those assignments
> > > > in the caller and directly call parallel_vacuum_all_indexes()?
> > >
> > > The reason why I declare these two functions are: (1) the fields of
> > > ParallelVacuumState are not exposed and (2) bulk-deletion and cleanup
> > > require different arguments (estimated_count is required only by
> > > cleanup).  So if we expose the fields of ParallelVacuumState, the
> > > caller can do those assignments and directly call
> > > parallel_vacuum_all_indexes(). But I'm not sure it's good if those
> > > assignments are the caller's responsibility.
> > >
> >
> > Okay, that makes sense. However, I am still not very comfortable with
> > the function naming suggested by Hou-San, do you have any thoughts on
> > that?
>
> I personally don't disagree with the names starting with
> "parallel_vacuum_*".
>

I don't have any strong opinion here but I prefer the name which makes
more sense in the context it is being used. OTOH, I see there is an
argument that it will be easier to follow and might appear consistent
if we use parallel_vacuum_*.

-- 
With Regards,
Amit Kapila.



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.