Re: [HACKERS] Block level parallel vacuum

On Wed, 6 Nov 2019, 20:07 Masahiko Sawada, <masahiko(dot)sawada(at)2ndquadrant(dot)com>
wrote:

> On Wed, 6 Nov 2019 at 20:29, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Wed, Nov 6, 2019 at 3:50 PM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > >
> > > On Wed, 6 Nov 2019 at 18:42, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
> wrote:
> > > >
> > > > On Wed, Nov 6, 2019 at 2:01 PM Mahendra Singh <mahi6run(at)gmail(dot)com>
> wrote:
> > > > >
> > > > > Hi
> > > > > I took all attached patches(v32-01 to v32-4) and one Dilip's patch
> from "Questions/Observations related to Gist vacuum" mail thread. On the
> top of all these patches, I created one more patch to test parallel vacuum
> functionally for all existence test suite.
> > >
> > > Thank you for looking at this patch!
> > >
> > > > > For reference, I am attaching patch.
> > > > >
> > > > > What does this patch?
> > > > > As we know that if we give parallel option with vacuum, then only
> we are vacuuming using parallel workers. So to test, I used existence guc
> force_parallel_mode and tested parallel vacuuming.
> > > > >
> > > > > If force_parallel_mode is set as regress, then if parallel option
> is not given with vacuum, I am forcing to use parallel workers for vacuum.
> If there is only one index and parallel degree is not given with vacuum(or
> parallel option is not given), and force_parallel_mode = regress, then I am
> launching one parallel worker(I am not doing work by leader in this case),
> but if there is more than one index, then i am using leader as a worker for
> one index and launching workers for all other indexes.
> > > > >
> > > > > After applying this patch and setting force_parallel_mode =
> regress, all test cases are passing (make-check world)
> > > > >
> > > > > I have some questions regarding my patch. Should we do vacuuming
> using parallel workers even if force_parallel_mode is set as on, or we
> should use new GUC to test parallel worker vacuum for existence test suite?
> > > >
> > > > IMHO, with force_parallel_mode=on we don't need to do anything here
> > > > because that is useful for normal query parallelism where if the user
> > > > thinks that the parallel plan should have been selected by the planer
> > > > but planer did not select the parallel plan then the user can force
> > > > and check. But, vacuum parallelism is itself forced by the user so
> > > > there is no point in doing it with force_parallel_mode=on.
> > >
> > > Yeah I think so too. force_parallel_mode is a planner parameter and
> > > parallel vacuum can be forced by vacuum option.
> > >
> > > > However,
> > > > force_parallel_mode=regress is useful for testing the vacuum with an
> > > > existing test suit.
> > >
> > > If we want to control the leader participation by GUC parameter I
> > > think we would need to have another GUC parameter rather than using
> > > force_parallel_mode.
> > I think the purpose is not to disable the leader participation,
> > instead, I think the purpose of 'force_parallel_mode=regress' is that
> > without changing the existing test suit we can execute the existing
> > vacuum commands in the test suit with the worker. I did not study the
> > patch but the idea should be that if "force_parallel_mode=regress"
> > then normal vacuum command should be executed in parallel by using 1
> > worker.
>
> Oh I got it. Considering the current parallel vacuum design I'm not
> sure that we can cover more test cases by forcing parallel vacuum
> during existing test suite because most of these would be tables with
> several indexes and one index vacuum cycle.

Oh sure, but still it would be good to get them tested with the parallel
vacuum.

> It might be better to add
> more test cases for parallel vacuum.
>

I agree that it would be good to add additional test cases.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com