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. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
bufferusage_vacuum_v3.patch
Description: Binary data
bufferusage_create_index_v2.patch
Description: Binary data
