From 15dec1e572ac4da0540251253c3c219eadf46a83 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 24 Feb 2022 17:21:45 -0800 Subject: [PATCH v9 4/4] Avoid setting a page all-visible but not all-frozen. This is pretty much an addendum to the work in the "Make page-level characteristics drive freezing" commit. It has been broken out like this because I'm not even sure if it's necessary. It seems like we might want to be paranoid about losing out on the chance to advance relfrozenxid in non-aggressive VACUUMs, though. The only test that will trigger this case is the "freeze-the-dead" isolation test. It's incredibly narrow. On the other hand, why take a chance? All it takes is one heap page that's all-visible (and not also all-frozen) nestled between some all-frozen heap pages to lose out on relfrozenxid advancement. The SKIP_PAGES_THRESHOLD stuff won't save us then [1]. [1] For context see commit bf136cf6e3 -- SKIP_PAGES_THRESHOLD is specifically concerned with relfrozenxid advancement in non-aggressive VACUUMs, and always has been. This isn't directly documented right now. --- src/backend/access/heap/vacuumlazy.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index b2d3b039d..5eede8c55 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1981,6 +1981,26 @@ retry: } #endif + /* + * Since OldestXmin and OldestMxact are not absolutely precise, there is a + * tiny chance that we will consider the page all-visible while not also + * considering it all-frozen (having frozen the page with the expectation + * that that would render it all-frozen). This can happen when there is a + * MultiXact containing XIDs from before and after OldestXmin, for + * example. This risks making relfrozenxid advancement by future + * non-aggressive VACUUMs impossible, which is a heavy price to pay just + * to be able to avoid accessing one single isolated heap page. + * + * We could just live with this, but it seems prudent to avoid the problem + * instead. And so we deliberately throw away the opportunity to set such + * a page all-visible instead of allowing this case. + * + * XXX What about the lazy_vacuum_heap_page/heap_page_is_all_visible path, + * which could still set the page just all-visible when that happens? + */ + if (prunestate->all_visible && !prunestate->all_frozen) + prunestate->all_visible = false; + /* * Now save details of the LP_DEAD items from the page in vacrel */ -- 2.30.2