Re: [HACKERS] Block level parallel vacuum

On Wed, Feb 13, 2019 at 9:32 PM Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> wrote:
>
>
> On Sat, Feb 9, 2019 at 11:47 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>> On Tue, Feb 5, 2019 at 12:14 PM Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> wrote:
>> >
>> >
>> > On Fri, Feb 1, 2019 at 8:19 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> >>
>> >>
>> >> The passing stats = NULL to amvacuumcleanup and ambulkdelete means the
>> >> first time execution. For example, btvacuumcleanup skips cleanup if
>> >> it's not NULL.In the normal vacuum we pass NULL to ambulkdelete or
>> >> amvacuumcleanup when the first time calling. And they store the result
>> >> stats to the memory allocated int the local memory. Therefore in the
>> >> parallel vacuum I think that both worker and leader need to move it to
>> >> the shared memory and mark it as updated as different worker could
>> >> vacuum different indexes at the next time.
>> >
>> >
>> > OK, understood the point. But for btbulkdelete whenever the stats are NULL,
>> > it allocates the memory. So I don't see a problem with it.
>> >
>> > The only problem is with btvacuumcleanup, when there are no dead tuples
>> > present in the table, the btbulkdelete is not called and directly the btvacuumcleanup
>> > is called at the end of vacuum, in that scenario, there is code flow difference
>> > based on the stats. so why can't we use the deadtuples number to differentiate
>> > instead of adding another flag?
>>
>> I don't understand your suggestion. What do we compare deadtuples
>> number to? Could you elaborate on that please?
>
>
> The scenario where the stats should pass NULL to btvacuumcleanup function is
> when there no dead tuples, I just think that we may use that deadtuples structure
> to find out whether stats should pass NULL or not while avoiding the extra
> memcpy.
>

Thank you for your explanation. I understood. Maybe I'm worrying too
much but I'm concernced compatibility; currently we handle indexes
individually. So if there is an index access method whose ambulkdelete
returns NULL at the first call but returns a palloc'd struct at the
second time or other, that doesn't work fine.

The documentation says that passed-in 'stats' is NULL at the first
time call of ambulkdelete but doesn't say about the second time or
more. Index access methods may expect that the passed-in 'stats' is
the same as what they has returned last time. So I think to add an
extra flag for keeping comptibility.

>>
>> > And also this scenario is not very often, so avoiding
>> > memcpy for normal operations would be better. It may be a small gain, just
>> > thought of it.
>> >
>>
>> This scenario could happen periodically on an insert-only table.
>> Additional memcpy is executed once per indexes in a vacuuming but I
>> agree that the avoiding memcpy would be good.
>
>
> Yes, understood. If possible removing the need of memcpy would be good.
> The latest patch doesn't apply anymore. Needs a rebase.
>

Thank you. Attached the rebased patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center