Thank you for reviewing!
Attached updated patch.
On Thu, Mar 10, 2016 at 3:37 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Mar 9, 2016 at 9:09 AM, Masahiko Sawada
> <sawada.mshk@gmail.com> wrote: Attached latest 2 patches.
>> * 000 patch : Incorporated the review comments and made rewriting
>> logic more clearly.
>
> That's better, thanks. But your comments don't survive pgindent.
> After running pgindent, I get this:
>
> + /*
> + * These old_* variables point to old visibility map page.
> + *
> + * cur_old : Points to current position on old
> page. blkend_old :
> + * Points to end of old block. break_old : Points to
> old page break
> + * position for rewriting a new page. After wrote a
> new page, old_end
> + * proceeds rewriteVmBytesPerPgae bytes.
> + */
>
> You need to either surround this sort of thing with dashes to make
> pgindent ignore it, or, probably better, rewrite it using complete
> sentences that together form a paragraph.
Fixed.
>
> + Oid pg_database_oid; /* OID of
> pg_database relation */
>
> Not used anywhere?
Fixed.
> Instead of vm_need_rewrite, how about vm_must_add_frozenbit?
Fixed.
> Can you explain the changes to test.sh?
Current regression test scenario is,
1. Do 'make check' on pre-upgrade cluster
2. Dump relallvisible values of all relation in pre-upgrade cluster to
vm_test1.txt
3. Do pg_upgrade
4. Do analyze (not vacuum), dump relallvisibile values of all relation
in post-upgrade cluster to vm_test2.txt
5. Compare between vm_test1.txt and vm_test2.txt
That is, regression test compares between relallvisible values in
pre-upgrade cluster and post-upgrade cluster.
But because test.sh always uses pre/post clusters with same catalog
version, I realized that we cannot ensure that visibility map
rewriting is processed successfully on test.sh framework.
Rewriting visibility map never be executed.
We might need to have another framework for rewriting visibility map page..
Regards,
--
Masahiko Sawada