On Tue, Mar 31, 2020 at 10:44 AM Masahiko Sawada <masahiko.saw...@2ndquadrant.com> wrote: > > On Tue, 31 Mar 2020 at 12:58, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Mar 30, 2020 at 12:31 PM Masahiko Sawada > > <masahiko.saw...@2ndquadrant.com> wrote: > > > > > > The patch for vacuum conflicts with recent changes in vacuum. So I've > > > attached rebased one. > > > > > > > + /* > > + * 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]); > > + > > > > This should be done for launched workers aka > > lps->pcxt->nworkers_launched. I think a similar problem exists in > > create index related patch. > > You're right. Fixed in the new patches. > > On Mon, 30 Mar 2020 at 17:00, Julien Rouhaud <rjuju...@gmail.com> wrote: > > > > 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? > > We can do that but I was not sure if it's good since other codes > around there don't use that. So I'd like to leave it for committers. > It's a trivial change.
I have reviewed the patch and the patch looks fine to me. One minor comment /+ /* Points to buffer usage are in DSM */ + BufferUsage *buffer_usage; + /buffer usage are in DSM / buffer usage area in DSM -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
