On Wed, 15 Jan 2020 at 17:27, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jan 15, 2020 at 10:05 AM Masahiko Sawada > <masahiko.saw...@2ndquadrant.com> wrote: > > > > Thank you for updating the patch! I have a few small comments. > > > > I have adapted all your changes, fixed the comment by Mahendra related > to initializing parallel state only when there are at least two > indexes. Additionally, I have changed a few comments (make the > reference to parallel vacuum consistent, at some places we were > referring it as 'parallel lazy vacuum' and at other places it was > 'parallel index vacuum'). > > > The > > rest looks good to me. > > > > Okay, I think the patch is in good shape. I am planning to read it a > few more times (at least 2 times) and then probably will commit it > early next week (Monday or Tuesday) unless there are any major > comments. I have already committed the API patch (4d8a8d0c73). >
Hi, Thanks Amit for fixing review comments. I reviewed v48 patch and below are some comments. 1. + * based on the number of indexes. -1 indicates a parallel vacuum is I think, above should be like "-1 indicates that parallel vacuum is" 2. +/* Variables for cost-based parallel vacuum */ At the end of comment, there is 2 spaces. I think, it should be only 1 space. 3. I think, we should add a test case for parallel option(when degree is not specified). *Ex:* postgres=# VACUUM (PARALLEL) tmp; ERROR: parallel option requires a value between 0 and 1024 LINE 1: VACUUM (PARALLEL) tmp; ^ postgres=# Because above error is added in this parallel patch, so we should have test case for this to increase code coverage. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com