From bf0d21e6172ba049a51016215e31b5d198ca9abf Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 27 Nov 2013 13:10:16 +0200 Subject: [PATCH] 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. --- src/backend/commands/vacuumlazy.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index f4bffb7d90..1d2472e991 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -166,7 +166,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, BlockNumber possibly_freeable; PGRUsage ru0; TimestampTz starttime = 0; - 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; @@ -209,6 +210,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. + * + * 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. * @@ -245,9 +261,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, new_rel_tuples = vacrelstats->old_rel_tuples; } - new_frozen_xid = FreezeLimit; - if (vacrelstats->scanned_pages < vacrelstats->rel_pages) - new_frozen_xid = InvalidTransactionId; + new_frozen_xid = scanned_all ? FreezeLimit : InvalidTransactionId; vac_update_relstats(onerel, new_rel_pages, new_rel_tuples, -- 2.40.0