Re: [HACKERS] Block level parallel vacuum

On Wed, 27 Nov 2019 at 13:28, Mahendra Singh <mahi6run@gmail.com> wrote:
>
> On Wed, 27 Nov 2019 at 08:14, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> 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.
>>
>> [1] -
https://www.postgresql.org/message-id/CAFiTN-uQY%2BB%2BCLb8W3YYdb7XmB9hyYFXkAy3C7RY%3D-YSWRV1DA%40mail.gmail.com
>>
>
> I did some testing on the top of v33 patch set. By debugging, I was able to hit one assert in
lazy_parallel_vacuum_or_cleanup_indexes.
> TRAP: FailedAssertion("nprocessed == nindexes_remains", File: "vacuumlazy.c", Line: 2099)
>
> I further debugged and found that this assert is not valid in all the cases. Here, nprocessed can be less than
nindexes_remainsin some cases because it is  possible that parallel worker is launched for vacuum and idx count is
incrementedin vacuum_or_cleanup_indexes_worker for particular index  but work is still not
finished(lvshared->nprocessedis 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
becausethis USE_ASSERT_CHECKING is used in 3 places in 20-30 code lines. 

Thank you for testing!

Yes, I think your analysis is right. I've removed the assertion in v35
patch that I've just posted[1].

[1] https://www.postgresql.org/message-id/CA%2Bfd4k5oAuGuwZ9XaOTv%2BcTU8-dmA3RjpJ%2Bi4x5kt9VbAFse1w%40mail.gmail.com


Regards,

--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services