On Fri, Jul 1, 2016 at 10:09 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> Ah, I see. So your suggestion is to do this job in lazy_scan_heap()
>>> when scanning each block, and then to issue a WARNING and clear the
>>> visibility map. Indeed that's better. I guess I need to take a closer
>>> look at vacuumlazy.c. See attached for example, but that's perhaps not
>>> something to have in 9.6 as that's more a micro-optimization than
>>> anything else.
>>
>> Right, something like that. I think Andres actually wants something
>> like this in 9.6, and I'm inclined to think it might be a good idea,
>> too. I think there should probably be a test for
>> all_visible_according_to_vm at the beginning of that so that we don't
>> add more visibility map checks for pages where we already know the VM
>> bit can't possibly be set.
>>
>> Other opinions on the concept or the patch?
>
> +1 on the idea.
>
> + PageClearAllVisible(page);
> + MarkBufferDirty(buf);
>
> What is the need to clear the Page level bit, if it is already
> cleared, doesn't '!all_frozen' indicate that?
No, I don't think so. I think all_frozen indicates whether we think
that all tuples on the page qualify as fully frozen. I don't think it
tells us anything about whether PD_ALL_VISIBLE is set on the page.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company