From: Julien Rouhaud <rjuju123@gmail.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_stat_statements issue with parallel maintenance (Was Re: WAL usage calculation patch)
Date: Mon, 30 Mar 2020 10:00:05 +0200
Message-ID: <20200330080005.GC79261@nol> (raw)
In-Reply-To: <CA+fd4k5W+AXFFfAD=UucYzunxDOCY57qVH0qO_5dHhr7SXYecA@mail.gmail.com>
References: <20200328133827.GA12854@nol>
<20200328151721.GB12854@nol>
<CAA4eK1Ke+LxUSWFdD=Q2=xRLvv0jC4i8EJYdfEYk9eM86yy-aw@mail.gmail.com>
<CA+fd4k6XxNw8AMf0wHAwn9tmumZxShB2ue8pXYaVDc9fm1spyQ@mail.gmail.com>
<CA+fd4k6DAAoCv+q_sGcHH2PPpN6AsHCS-LFYNJq9NaZtQHL2Ew@mail.gmail.com>
<CAOBaU_Yc5OFKDYG_EOjvo3OGsP8DK_rHXj8bMQg_h6u0dc0zUw@mail.gmail.com>
<CAA4eK1K1Znh7ZBP+Jgpue+hHraDi5=R7CD5dcm9Q9DL8b0vEyQ@mail.gmail.com>
<CA+fd4k6YCYn9WT_BwbgxhmY8x7GLnks6cTeTutVBrdNW2K+Xbw@mail.gmail.com>
<CA+fd4k6WS3h_y+OKAm5b=rYUzA3woRTEKp-_X6oqL71_Ac=iNQ@mail.gmail.com>
<CA+fd4k5W+AXFFfAD=UucYzunxDOCY57qVH0qO_5dHhr7SXYecA@mail.gmail.com>
On Mon, Mar 30, 2020 at 04:01:18PM +0900, Masahiko Sawada wrote:
> On Mon, 30 Mar 2020 at 15:46, Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > On Sun, 29 Mar 2020 at 20:44, Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> > >
> > > > > > I think we need to change parallel maintenance commands so that they
> > > > > > report buffer usage like what ParallelQueryMain() does; prepare to
> > > > > > track buffer usage during query execution by
> > > > > > InstrStartParallelQuery(), and report it by InstrEndParallelQuery()
> > > > > > after parallel maintenance command. To report buffer usage of parallel
> > > > > > maintenance command correctly, I'm thinking that we can (1) change
> > > > > > parallel create index and parallel vacuum so that they prepare
> > > > > > gathering buffer usage, or (2) have a common entry point for parallel
> > > > > > maintenance commands that is responsible for gathering buffer usage
> > > > > > and calling the entry functions for individual maintenance command.
> > > > > > I'll investigate it more in depth.
> > > > >
> > > [...]
> >
> > I've attached two patches fixing this issue for parallel index
> > creation and parallel vacuum. These approaches take the same approach;
> > we allocate DSM to share buffer usage and the leader gathers them,
> > described as approach (1) above. I think this is a straightforward
> > approach for this issue. We can create a common entry point for
> > parallel maintenance command that is responsible for gathering buffer
> > usage as well as sharing query text etc. But it will accompany
> > relatively big change and it might be overkill at this stage. We can
> > discuss that and it will become an item for PG14.
> >
>
> The patch for vacuum conflicts with recent changes in vacuum. So I've
> attached rebased one.
Thanks Sawada-san!
Just minor nitpicking:
+ int i;
Assert(!IsParallelWorker());
Assert(ParallelVacuumIsActive(lps));
@@ -2166,6 +2172,13 @@ lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
/* Wait for all vacuum workers to finish */
WaitForParallelWorkersToFinish(lps->pcxt);
+ /*
+ * Next, accumulate buffer usage. (This must wait for the workers to
+ * finish, or we might get incomplete data.)
+ */
+ for (i = 0; i < nworkers; i++)
+ InstrAccumParallelQuery(&lps->buffer_usage[i]);
We now allow declaring a variable in those loops, so it may be better to avoid
declaring i outside the for scope?
Other than that both patch looks good to me and a good fit for packpatching. I
also did some testing on VACUUM and CREATE INDEX and it works as expected.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rjuju123@gmail.com, masahiko.sawada@2ndquadrant.com, amit.kapila16@gmail.com, dilipbalaut@gmail.com, robertmhaas@gmail.com
Subject: Re: pg_stat_statements issue with parallel maintenance (Was Re: WAL usage calculation patch)
In-Reply-To: <20200330080005.GC79261@nol>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox