Re: parallel vacuum comments

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(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-11-29 03:38:08
Message-ID: CAD21AoC23YvzxyWmwq1O9am4jM5uZa_aH7w=KA-s-nsq95Modg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 24, 2021 at 5:54 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Nov 24, 2021 at 12:16 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Wed, Nov 24, 2021 at 1:34 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Nov 24, 2021 at 7:43 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > >
> > > > On Mon, Nov 22, 2021 at 1:48 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > > > On Fri, Nov 19, 2021 at 7:55 AM houzj(dot)fnst(at)fujitsu(dot)com
> > > > > <houzj(dot)fnst(at)fujitsu(dot)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_*.

Maybe we can start with using parallel_vacuum_*. We can change them
later if there is an argument.

I've attached an updated patch. I don't update the terminology in
vacuum that we're discussing on another thread[1].

Regards,

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

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

Attachment Content-Type Size
parallel_vacuum_refactor_v4.patch application/octet-stream 87.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-11-29 03:38:19 Re: pg_upgrade and publication/subscription problem
Previous Message Justin Pryzby 2021-11-29 03:08:33 GUC flags