On Tue, 28 Jan 2020 at 12:32, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Jan 28, 2020 at 12:04 PM Mahendra Singh Thalor > <mahi6...@gmail.com> wrote: > > > > On Tue, 28 Jan 2020 at 08:14, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Tue, Jan 28, 2020 at 2:13 AM Mahendra Singh Thalor > > > <mahi6...@gmail.com> wrote: > > > > > > > > On Sat, 25 Jan 2020 at 12:11, Amit Kapila <amit.kapil...@gmail.com> > > > > wrote: > > > > > > > > > > On Fri, Jan 24, 2020 at 4:58 PM Mahendra Singh Thalor > > > > > <mahi6...@gmail.com> wrote: > > > > > > > > > > > > On Thu, 23 Jan 2020 at 15:32, Mahendra Singh Thalor > > > > > > <mahi6...@gmail.com> wrote: > > > > > > > > > > > > > > On Wed, 22 Jan 2020 at 12:48, Masahiko Sawada > > > > > > > <masahiko.saw...@2ndquadrant.com> wrote: > > > > > > > > > > > > > > > > Attached the updated version patch. > > > > > > > > > > > > > > Thanks Sawada-san for the re-based patch. > > > > > > > > > > > > > > I reviewed and tested this patch. Patch looks good to me. > > > > > > > > > > > > As offline, suggested by Amit Kapila, I verified vacuumdb "-P" > > > > > > option > > > > > > functionality with older versions(<13) and also I tested vacuumdb by > > > > > > giving "-j" option with "-P". All are working as per expectation > > > > > > and I > > > > > > didn't find any issue with these options. > > > > > > > > > > > > > > > > I have made few modifications in the patch. > > > > > > > > > > 1. I think we should try to block the usage of 'full' and 'parallel' > > > > > option in the utility rather than allowing the server to return an > > > > > error. > > > > > 2. It is better to handle 'P' option in getopt_long in the order of > > > > > its declaration in long_options array. > > > > > 3. Added an Assert for server version while handling of parallel > > > > > option. > > > > > 4. Added a few sentences in the documentation. > > > > > > > > > > What do you guys think of the attached? > > > > > > > > > > > > > I took one more review round. Below are some review comments: > > > > > > > > 1. > > > > -P, --parallel=PARALLEL_DEGREE do parallel vacuum > > > > > > > > I think, "do parallel vacuum" should be modified. Without specifying > > > > -P, we are still doing parallel vacuum so we can use like "degree for > > > > parallel vacuum" > > > > > > > > > > I am not sure if 'degree' makes it very clear. How about "use this > > > many background workers for vacuum, if available"? > > > > If background workers are many, then automatically, we are using them(by > > default parallel vacuum). This option is to put limit on background > > workers(limit for vacuum workers) to be used by vacuum process. > > > > I don't think that the option is just to specify the max limit because > that is generally controlled by guc parameters. This option allows > users to specify the number of workers for the cases where he has more > knowledge about the size/type of indexes. In some cases, the user > might be able to make a better decision and that was the reason we > have added this option in the first place. > > > So I think, we can use "max parallel vacuum workers (by default, based on > > no. of indexes)" or "control parallel vacuum workers" > > > > Hmm, I feel what I suggested is better because of the above explanation.
Agreed. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
