On Fri, 22 Nov 2019 at 10:19, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Nov 20, 2019 at 11:01 AM Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > I've attached the latest version patch set. The patch set includes all
> > discussed points regarding index AM options as well as shared cost
> > balance. Also I added some test cases used all types of index AM.
> >
>
> I have reviewed the first patch and made a number of modifications
> that include adding/modifying comments, made some corrections and
> modifications in the documentation. You can find my changes in
> v33-0001-delta-amit.patch. See, if those look okay to you, if so,
> please include those in the next version of the patch. I am attaching
> both your version of patch and delta changes by me.
Thank you.
All changes look good to me. But after changed the 0002 patch the two
macros for parallel vacuum options (VACUUM_OPTIONS_SUPPORT_XXX) is no
longer necessary. So we can remove them and can add if we need them
again.
>
> One comment on v33-0002-Add-parallel-option-to-VACUUM-command:
>
> + /* Estimate size for shared information -- PARALLEL_VACUUM_KEY_SHARED */
> + est_shared = MAXALIGN(add_size(SizeOfLVShared, BITMAPLEN
> (nindexes)));
> ..
> + shared->offset = add_size(SizeOfLVShared, BITMAPLEN(nindexes));
>
> Here, don't you need to do MAXALIGN to set offset as we are computing
> it that way while estimating shared memory? If not, then probably,
> some comments are required to explain it.
You're right. Will fix it.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services