On Wed, 6 Nov 2019, 20:07 Masahiko Sawada, <masahiko.saw...@2ndquadrant.com> wrote:
> On Wed, 6 Nov 2019 at 20:29, Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Wed, Nov 6, 2019 at 3:50 PM Masahiko Sawada > > <masahiko.saw...@2ndquadrant.com> wrote: > > > > > > On Wed, 6 Nov 2019 at 18:42, Dilip Kumar <dilipbal...@gmail.com> > wrote: > > > > > > > > On Wed, Nov 6, 2019 at 2:01 PM Mahendra Singh <mahi6...@gmail.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
