Re: Setting visibility map in VACUUM's second phase

On Fri, Dec 7, 2012 at 9:21 AM, Pavan Deolasee <pavan.deolasee@gmail.com> wrote:
> On Thu, Dec 6, 2012 at 11:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>>
>> I think taking a second whack at setting the visibility bit is a fine
>> idea, but let's drop all the rest of this premature optimization.
>>
>
> Fair enough. I thought about doing it that way but was worried that an
> additional page scan will raise eyebrows. While it does not affect the
> common case because we would have done that scan anyways in the
> subsequent vacuum, but in the worst case where most of the pages not
> remain all-visible by the time we come back to the second phase of
> vacuum, this additional line pointer scan will add some overhead. With
> couple of pluses for the approach, I won't mind doing it this way
> though.
>

Revised patch attached. This time much less invasive. I added a new
function heap_page_is_all_visible() to check if the given page is
all-visible and also return the visibility cutoff xid. Couple of
things:

- We use the same OldestXmin computed in the first phase for
visibility checks. We can potentially recompute this at the start of
second phase. I wasn't convinced thats needed or even correct

- It will be a good idea to consolidate the page scanning in a single
place to avoid code duplication. But may be will do that some other
day.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee