On Fri, Dec 6, 2019 at 12:55 AM Mahendra Singh <mahi6...@gmail.com> wrote: > > On Thu, 5 Dec 2019 at 19:54, Robert Haas <robertmh...@gmail.com> wrote: > > > > [ Please trim excess quoted material from your replies. ] > > > > On Thu, Dec 5, 2019 at 12:27 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > I agree that there is no point is first to spawn more workers to get > > > the work done faster and later throttle them. Basically, that will > > > lose the whole purpose of running it in parallel. > > > > Right. I mean if you throttle something that would have otherwise > > kept 3 workers running full blast back to the point where it uses the > > equivalent of 2.5 workers, that might make sense. It's a little > > marginal, maybe, but sure. But once you throttle it back to <= 2 > > workers, you're just wasting resources. > > > > I think my concern here is ultimately more about usability than > > whether or not we allow throttling. I agree that there are some > > possible cases where throttling a parallel vacuum is useful, so I > > guess we should support it. But I also think there's a real risk of > > people not realizing that throttling is happening and then being sad > > because they used parallel VACUUM and it was still slow. I think we > > should document explicitly that parallel VACUUM is still potentially > > throttled and that you should consider setting the cost delay to a > > higher value or 0 before using it. > > > > We might even want to add a FAST option (or similar) to VACUUM that > > makes it behave as if vacuum_cost_delay = 0, and add something to the > > examples section for VACUUM that suggests e.g. > > > > VACUUM (PARALLEL 3, FAST) my_big_table > > Vacuum my_big_table with 3 workers and with resource throttling > > disabled for maximum performance. > > > > Please find some review comments for v35 patch set > > 1. > + /* Return immediately when parallelism disabled */ > + if (max_parallel_maintenance_workers == 0) > + return 0; > + > Here, we should add check of max_worker_processes because if > max_worker_processes is set as 0, then we can't launch any worker so > we should return from here. > > 2. > + /* cap by max_parallel_maintenace_workers */ > + parallel_workers = Min(parallel_workers, > max_parallel_maintenance_workers); > + > Here also, we should consider max_worker_processes to calculate > parallel_workers. (by default, max_worker_processes = 8)
IMHO, it's enough to cap with max_parallel_maintenace_workers. So I think it's the user's responsibility to keep max_parallel_maintenace_workers under parallel_workers limit. And, if the user fails to set max_parallel_maintenace_workers under the parallel_workers or enough workers are not available then LaunchParallel worker will take care. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
