On Tue, Mar 8, 2016 at 7:26 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Regarding pg_visibility module, I'd like to share some bugs and
> propose to add a relation type condition to each functions.
OK, thanks.
> Including it, I've attached remaining 2 patches; one is removing page
> conversion code from pg_upgarde, and another is supporting pg_upgrade
> for frozen bit.
Committed 001 with minor tweaks.
I find rewrite_vm_table to be pretty opaque. There's not even a
comment explaining what it is supposed to do. And I wonder why we
really need to be this efficient about it anyway. Like, would it be
too expensive to just do this:
for (i = 0; i < BITS_PER_BYTE; ++i) if ((old & (1 << i)) != 0) new |= 1 << (2 * i);
And how about adding some more comments explaining why we are doing
this rewriting, like this:
In versions of PostgreSQL prior to catversion 201602181, PostgreSQL's
visibility map included one bit per heap page; it now includes two.
When upgrading a cluster from before that time to a current PostgreSQL
version, we could refuse to copy visibility maps from the old cluster
to the new cluster; the next VACUUM would recreate them, but at the
price of scanning the entire table. So, instead, we rewrite the old
visibility maps in the new format. That way, the all-visible bit
remains set for the pages for which it was set previously. The
all-frozen bit is never set by this conversion; we leave that to
VACUUM.
Also, I'm slightly perplexed by the fact that I can't see how this
code succeeds in turning each page into two pages, which is something
that it seems like it would need to do. Wouldn't we need to write out
the old page header twice, one for the first of the two new pages and
again for the second? I probably need more caffeine here, so please
tell me what I'm missing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
