Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 82b43f7

Browse files
committed
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 b4b6923. Backpatch this fix down to 8.4, like that commit. Andres Freund, with some modifications by me.
1 parent 2390f2b commit 82b43f7

File tree

1 file changed

+21
-10
lines changed

1 file changed

+21
-10
lines changed

src/backend/commands/vacuumlazy.c

+21-10
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
178178
int usecs;
179179
double read_rate,
180180
write_rate;
181-
bool scan_all;
181+
bool scan_all; /* should we scan all pages? */
182+
bool scanned_all; /* did we actually scan all pages? */
182183
TransactionId freezeTableLimit;
183184
BlockNumber new_rel_pages;
184185
double new_rel_tuples;
@@ -225,6 +226,21 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
225226
/* Done with indexes */
226227
vac_close_indexes(nindexes, Irel, NoLock);
227228

229+
/*
230+
* Compute whether we actually scanned the whole relation. If we did, we
231+
* can adjust relfrozenxid and relminmxid.
232+
*
233+
* NB: We need to check this before truncating the relation, because that
234+
* will change ->rel_pages.
235+
*/
236+
if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
237+
{
238+
Assert(!scan_all);
239+
scanned_all = false;
240+
}
241+
else
242+
scanned_all = true;
243+
228244
/*
229245
* Optionally truncate the relation.
230246
*
@@ -254,8 +270,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
254270
* is all-visible we'd definitely like to know that. But clamp the value
255271
* to be not more than what we're setting relpages to.
256272
*
257-
* Also, don't change relfrozenxid if we skipped any pages, since then we
258-
* don't know for certain that all tuples have a newer xmin.
273+
* Also, don't change relfrozenxid/relminmxid if we skipped any pages,
274+
* since then we don't know for certain that all tuples have a newer xmin.
259275
*/
260276
new_rel_pages = vacrelstats->rel_pages;
261277
new_rel_tuples = vacrelstats->new_rel_tuples;
@@ -269,13 +285,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
269285
if (new_rel_allvisible > new_rel_pages)
270286
new_rel_allvisible = new_rel_pages;
271287

272-
new_frozen_xid = FreezeLimit;
273-
if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
274-
new_frozen_xid = InvalidTransactionId;
275-
276-
new_min_multi = MultiXactCutoff;
277-
if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
278-
new_min_multi = InvalidMultiXactId;
288+
new_frozen_xid = scanned_all ? FreezeLimit : InvalidTransactionId;
289+
new_min_multi = scanned_all ? MultiXactCutoff : InvalidMultiXactId;
279290

280291
vac_update_relstats(onerel,
281292
new_rel_pages,

0 commit comments

Comments
 (0)