Re: Reviewing freeze map code

On Fri, Jul 1, 2016 at 7:52 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Fri, Jul 1, 2016 at 11:12 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>> 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.
>>
>
> Ah, you're right, I misunderstood.
>
> Attached updated patch incorporating your comments.
> I've changed it so that heap_xlog_lock clears vm flags if page is
> marked all frozen.
>

I think we should make a similar change in heap_lock_tuple API as
well. Also, currently by default heap_xlog_lock tuple tries to clear
the visibility flags, isn't it better to handle it as we do at all
other places (ex. see log_heap_update), by logging the information
about same. I think it is always advisable to log every action we
want replay to perform. That way, it is always easy to extend it
based on if some change is required only in certain cases, but not in
others.

Though, it is important to get the patch right, but I feel in the
meantime, it might be better to start benchmarking. AFAIU, even if
change some part of information while WAL logging it, the benchmark
results won't be much different.

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