Re: Reviewing freeze map code

On Fri, Jun 3, 2016 at 11:03 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> Attached patch optimises skipping pages logic so that blkno can jump to
>> next_unskippable_block directly while counting the number of all_visible
>> and all_frozen pages. So we can avoid double checking visibility map.
>
> I think this is 9.7 material.  This patch has already won the
> "scariest patch" tournament.  Changing the logic more than necessary
> at this late date seems like it just increases the scariness.  I think
> this is an opportunity for further optimization, not a defect.
>

I agree with you.
I'll submit this as a improvement for 9.7.
That patch also incorporates the following review comment.
We can push at least this fix.
>>         /*
>>          * Compute whether we actually scanned the whole relation. If we did, we
>>          * can adjust relfrozenxid and relminmxid.
>>          *
>>          * NB: We need to check this before truncating the relation, because that
>>          * will change ->rel_pages.
>>          */
>>
>> Comment is out-of-date now.

I'm address the review comment of 7087166 commit, and will post the patch.
And testing feature for freeze map is under the discussion.

Regards,

--
Masahiko Sawada