Re: [HACKERS] Vacuum: allow usage of more than 1GB of work mem

On Wed, Feb 1, 2017 at 11:55 PM, Claudio Freire <klaussfreire@gmail.com> wrote:
> On Wed, Feb 1, 2017 at 6:13 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> On Wed, Feb 1, 2017 at 10:02 PM, Claudio Freire <klaussfreire@gmail.com> wrote:
>>> On Wed, Feb 1, 2017 at 5:47 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>>> Thank you for updating the patch.
>>>>
>>>> Whole patch looks good to me except for the following one comment.
>>>> This is the final comment from me.
>>>>
>>>> /*
>>>>  *  lazy_tid_reaped() -- is a particular tid deletable?
>>>>  *
>>>>  *      This has the right signature to be an IndexBulkDeleteCallback.
>>>>  *
>>>>  *      Assumes dead_tuples array is in sorted order.
>>>>  */
>>>> static bool
>>>> lazy_tid_reaped(ItemPointer itemptr, void *state)
>>>> {
>>>>     LVRelStats *vacrelstats = (LVRelStats *) state;
>>>>
>>>> You might want to update the comment of lazy_tid_reaped() as well.
>>>
>>> I don't see the mismatch with reality there (if you consider
>>> "dead_tples array" in the proper context, that is, the multiarray).
>>>
>>> What in particular do you find out of sync there?
>>
>> The current lazy_tid_reaped just find a tid from a tid array using
>> bsearch but in your patch lazy_tid_reaped handles multiple tid arrays
>> and processing method become complicated. So I thought it's better to
>> add the description of this function.
>
> Alright, updated with some more remarks that seemed relevant

Thank you for updating the patch.

The patch looks good to me. There is no review comment from me.

Regards,

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