On Wed, Nov 27, 2019 at 12:52 AM Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote: > > > I've incorporated the comments I got so far including the above and > the memory alignment issue. >
Thanks, I will look into the new version. BTW, why haven't you posted 0001 patch (IndexAM API's patch)? I think without that we need to use the previous version for that. Also, I think we should post Dilip's patch related to Gist index [1] modifications for parallel vacuum or at least have a mention for that while posting a new version as without that even make check fails.
I further debugged and found that this assert is not valid in all the cases. Here, nprocessed can be less than nindexes_remains in some cases because it is possible that parallel worker is launched for vacuum and idx count is incremented in vacuum_or_cleanup_indexes_worker for particular index but work is still not finished(lvshared->nprocessed is not incremented yet) so in that case, nprocessed will be less than nindexes_remains. I think, we should remove this assert.
I have one comment for assert used variable:
+#ifdef USE_ASSERT_CHECKING + int nprocessed = 0; +#endif
I think, we can make above declaration as " int nprocessed PG_USED_FOR_ASSERTS_ONLY = 0" so that code looks good because this USE_ASSERT_CHECKING is used in 3 places in 20-30 code lines.
