database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-31 22:04:40 +03:00
Code Activity

Repair pg_upgrade's failure to preserve relfrozenxid for matviews.

Browse Source 
This oversight led to data corruption in matviews, manifesting as
"could not access status of transaction" before our most recent releases,
and "found xmin from before relfrozenxid" errors since then.

The proximate cause of the problem seems to have been confusion between
the task of preserving dropped-column status and the task of preserving
frozenxid status.  Those are required for distinct sets of relkinds,
and the reasoning was entirely undocumented in the source code.  In hopes
of forestalling future errors of the same kind, try to improve the
commentary in this area.

In passing, also improve the remarkably unhelpful comments around
pg_upgrade's set_frozenxids().  That's not actually buggy AFAICS,
but good luck figuring out what it does from the old comments.

Per report from Claudio Freire.  It appears that bug #14852 from Alexey
Ermakov is an earlier report of the same issue, and there may be other
cases that we failed to identify at the time.

Patch by me based on analysis by Andres Freund.  The bug dates back
to the introduction of matviews, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAGTBQpbrY9CdRGGhyBZ9yqY4jWaGC85rUF4X+R7d-aim=mBNsw@mail.gmail.com
Discussion: https://postgr.es/m/20171013115320.28049.86457@wrigleys.postgresql.org
This commit is contained in:
Tom Lane
2018-02-21 18:40:24 -05:00
parent ea6d67cf8e
commit d3b0a23a20
2 changed files with 47 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
contrib/pg_upgrade/pg_upgrade.c
View File

@ -438,13 +438,13 @@ static void
prepare_new_databases(void)
{
/*
* We set autovacuum_freeze_max_age to its maximum value so autovacuum
* does not launch here and delete clog files, before the frozen xids are
* set.
* Before we restore anything, set frozenxids of initdb-created tables.
*/
set_frozenxids(false);
/*
* Now restore global objects (roles and tablespaces).
*/
prep_status("Restoring global objects in the new cluster");
/*
@ -682,14 +682,25 @@ copy_clog_xlog_xid(void)
/*
* set_frozenxids()
*
* We have frozen all xids, so set relfrozenxid and datfrozenxid
* to be the old cluster's xid counter, which we just set in the new
* cluster. User-table frozenxid values will be set by pg_dumpall
* --binary-upgrade, but objects not set by the pg_dump must have
* proper frozen counters.
* This is called on the new cluster before we restore anything, with
* minmxid_only = false. Its purpose is to ensure that all initdb-created
* vacuumable tables have relfrozenxid/relminmxid matching the old cluster's
* xid/mxid counters. We also initialize the datfrozenxid/datminmxid of the
* built-in databases to match.
*
* As we create user tables later, their relfrozenxid/relminmxid fields will
* be restored properly by the binary-upgrade restore script. Likewise for
* user-database datfrozenxid/datminmxid. However, if we're upgrading from a
* pre-9.3 database, which does not store per-table or per-DB minmxid, then
* the relminmxid/datminmxid values filled in by the restore script will just
* be zeroes.
*
* Hence, with a pre-9.3 source database, a second call occurs after
* everything is restored, with minmxid_only = true. This pass will
* initialize all tables and databases, both those made by initdb and user
* objects, with the desired minmxid value. frozenxid values are left alone.
*/
static
void
static void
set_frozenxids(bool minmxid_only)
{
int dbnum;