Re: Reviewing freeze map code

On Thu, Jun 30, 2016 at 8:10 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Thu, Jun 30, 2016 at 3:12 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Thu, Jun 30, 2016 at 9:13 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>> On 2016-06-30 08:59:16 +0530, Amit Kapila wrote:
>>>> On Wed, Jun 29, 2016 at 10:30 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>>> > On 2016-06-29 19:04:31 +0530, Amit Kapila wrote:
>>>> >> There is nothing in this record which recorded the information about
>>>> >> visibility clear flag.
>>>> >
>>>> > I think we can actually defer the clearing to the lock release?
>>>>
>>>> How about the case if after we release the lock on page, the heap page
>>>> gets flushed, but not vm and then server crashes?
>>>
>>> In the released branches there's no need to clear all visible at that
>>> point. Note how heap_lock_tuple doesn't clear it at all. So we should be
>>> fine there, and that's the part where reusing an existing record is
>>> important (for compatibility).
>>>
>>
>> For back branches, I agree that heap_lock_tuple is sufficient,
>
> Even if we use heap_lock_tuple, If server crashed after flushed heap
> but not vm, after crash recovery the heap is still marked all-visible
> on vm.

So, in this case both vm and page will be marked as all_visible.

> This case could be happen even on released branched, and could make
> IndexOnlyScan returns wrong result?
>

Why do you think IndexOnlyScan will return wrong result? If the
server crash in the way as you described, the transaction that has
made modifications will anyway be considered aborted, so the result of
IndexOnlyScan should not be wrong.

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