Re: pg_upgrade bug found!

Noah Misch wrote:
> On Thu, Apr 07, 2011 at 10:21:06PM -0400, Bruce Momjian wrote:
> > Noah Misch wrote:
> > > 1) The pg_class.relfrozenxid that the TOAST table should have received
> > > ("true relfrozenxid") is still covered by available clog files.  Fixable
> > > with some combination of pg_class.relfrozenxid twiddling and "SET
> > > vacuum_freeze_table_age = 0; VACUUM toasttbl".
> > 
> > Right, VACUUM FREEZE.  I now see I don't need to set
> > vacuum_freeze_table_age if I use the FREEZE keyword, e.g. gram.y has:
> > 
> >     if (n->options & VACOPT_FREEZE)
> >     n->freeze_min_age = n->freeze_table_age = 0;
> 
> True; it just performs more work than strictly necessary.  We don't actually
> need earlier-than-usual freezing.  We need only ensure that the relfrozenxid
> will guide future VACUUMs to do that freezing early enough.  However, I'm not
> sure how to do that without directly updating relfrozenxid, so it's probably
> just as well to cause some extra work and stick to the standard interface.

Looking at the code, it seems VACUUM FREEZE will freeze any row it can,
and because we restarted the cluster after the upgrade, all the rows we
care about are visible or dead to all transactions.  pg_upgrade
certainly relies on that fact already.

> > > 2) The true relfrozenxid is no longer covered by available clog files.
> > > The fix for case 1 will get "file "foo" doesn't exist, reading as
> > > zeroes" log messages, and we will treat all transactions as uncommitted.
> > 
> > Uh, are you sure?  I think it would return an error message about a
> > missing clog file for the query;  here is a report of a case not related
> > to pg_upgrade:
> > 
> >     http://archives.postgresql.org/pgsql-admin/2010-09/msg00109.php
> 
> My statement was indeed incorrect.  (Was looking at the "reading as zeroes"
> message in slru.c, but it only applies during recovery.)

No problem.  Thanks for the review.

> > > Not generally fixable after that has happened.  We could probably
> > > provide a recipe for checking whether it could have happened given
> > > access to a backup from just before the upgrade.
> > 
> > The IRC folks pulled the clog files off of backups.
> 
> Since we do get the error after all, that should always be enough.

That was my thought too.

> > One concern I have is that existing heap tables are protecting clog
> > files, but once those are frozen, the system might remove clog files not
> > realizing it has to freeze the heap tables too.
> 
> Yes.  On a similar note, would it be worth having your prototype fixup script
> sort the VACUUM FREEZE calls in descending relfrozenxid order?

Good question.  I don't think the relfrozenxid ordering would make sense
because I think it is unlikely problems will happen while they are
running the script.  The other problem is that because of wraparound it
is unclear which xid is earliest.  What I did add was to order by the
oid, so at least the toast numbers are in order and people can more
easily track its progress.

New version;  I made some other small adjustments:
-- This script fixes data in pre-PG 9.0.4 and pre-8.4.8-- servers that was upgraded by pg_upgrade and pg_migrator.--
Runthe script using psql for every database in the cluster-- except 'template0', e.g.:--     psql -U postgres -a -f
pg_upgrade_fix.sqldbname-- This must be run from a writable directory.-- It will not lock any tables but will generate
I/O.--CREATETEMPORARY TABLE pg_upgrade_fix AS    SELECT 'VACUUM FREEZE pg_toast.' || c.relname || ';'    FROM
pg_classc, pg_namespace n     WHERE     c.relnamespace = n.oid AND         n.nspname = 'pg_toast' AND        c.relkind
='t'    ORDER by c.oid;\copy pg_upgrade_fix TO 'pg_upgrade_tmp.sql';\i pg_upgrade_tmp.sql
 


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +