On Mon, Mar 30, 2020 at 04:01:18PM +0900, Masahiko Sawada wrote: > On Mon, 30 Mar 2020 at 15:46, Masahiko Sawada > <masahiko.saw...@2ndquadrant.com> wrote: > > > > On Sun, 29 Mar 2020 at 20:44, Masahiko Sawada > > <masahiko.saw...@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.