Re: Reviewing freeze map code

On Fri, Jun 3, 2016 at 11:03 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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