Re: Documentation fixes for pg_visibility

On Fri, Jul 1, 2016 at 6:58 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Jun 29, 2016 at 1:42 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Tue, Jun 28, 2016 at 7:05 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Mon, Jun 27, 2016 at 5:56 PM, Michael Paquier
>>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>>>> Under what circumstances would you wish to check only one page of a relation?
>>>>
>>>> What I'd like to be able to do is to stop scanning the relation once
>>>> one defective tuple has been found: if there is at least one problem,
>>>> the whole vm needs to be rebuilt anyway. So this function could be
>>>> wrapped in a plpgsql function for example. It is more flexible than
>>>> directly modifying this function so as it stops at the first problem
>>>> stopped.
>>>
>>> I think most likely the best way to handle this is teach VACUUM to do
>>> PageClearAllVisible() and visibilitymap_clear() on any page where
>>> VM_ALL_FROZEN(onerel, blkno, &vmbuffer) && !all_frozen. This would go
>>> well with the existing code to clear incorrectly-set visibility map
>>> bits, and it would allow VACUUM (DISABLE_PAGE_SKIPPING) to serve the
>>> purpose you're talking about here, but more efficiently.
>>
>> 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?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com