Don't update relfrozenxid if any pages were skipped.

Vacuum recognizes that it can update relfrozenxid by checking whether it has
processed all pages of a relation. Unfortunately it performed that check
after truncating the dead pages at the end of the relation, and used the new
number of pages to decide whether all pages have been scanned. If the new
number of pages happened to be smaller or equal to the number of pages
scanned, it incorrectly decided that all pages were scanned.

This can lead to relfrozenxid being updated, even though some pages were
skipped that still contain old XIDs. That can lead to data loss due to xid
wraparounds with some rows suddenly missing. This likely has escaped notice
so far because it takes a large number (~2^31) of xids being used to see the
effect, while a full-table vacuum before that would fix the issue.

The incorrect logic was introduced by commit
b4b6923e03f4d29636a94f6f4cc2f5cf6298b8c8. Backpatch this fix down to 8.4,
like that commit.

Andres Freund, with some modifications by me.

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 11d0192931f7e88a332fff6b1346f652ac7d1935..93ea08512428485e654823a9305b95257b50bf35 100644 (file)
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -178,7 +178,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        int                     usecs;
        double          read_rate,
                                write_rate;
-       bool            scan_all;
+       bool            scan_all;               /* should we scan all pages? */
+       bool            scanned_all;    /* did we actually scan all pages? */
        TransactionId freezeTableLimit;
        BlockNumber new_rel_pages;
        double          new_rel_tuples;
@@ -225,6 +226,21 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        /* Done with indexes */
        vac_close_indexes(nindexes, Irel, NoLock);
 
+       /*
+        * 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.
+        */
+       if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
+       {
+               Assert(!scan_all);
+               scanned_all = false;
+       }
+       else
+               scanned_all = true;
+
        /*
         * Optionally truncate the relation.
         *
@@ -254,8 +270,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
         * is all-visible we'd definitely like to know that.  But clamp the value
         * to be not more than what we're setting relpages to.
         *
-        * Also, don't change relfrozenxid if we skipped any pages, since then we
-        * don't know for certain that all tuples have a newer xmin.
+        * Also, don't change relfrozenxid/relminmxid if we skipped any pages,
+        * since then we don't know for certain that all tuples have a newer xmin.
         */
        new_rel_pages = vacrelstats->rel_pages;
        new_rel_tuples = vacrelstats->new_rel_tuples;
@@ -269,13 +285,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        if (new_rel_allvisible > new_rel_pages)
                new_rel_allvisible = new_rel_pages;
 
-       new_frozen_xid = FreezeLimit;
-       if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
-               new_frozen_xid = InvalidTransactionId;
-
-       new_min_multi = MultiXactCutoff;
-       if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
-               new_min_multi = InvalidMultiXactId;
+       new_frozen_xid = scanned_all ? FreezeLimit : InvalidTransactionId;
+       new_min_multi = scanned_all ? MultiXactCutoff : InvalidMultiXactId;
 
        vac_update_relstats(onerel,
                                                new_rel_pages,