From b37a0832396414e8469d4ee4daea33396bde39b0 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 3 Jan 2023 10:08:55 -0800 Subject: [PATCH] Refine the definition of page-level freezing. Improve comments added by commit 1de58df4 which describe the lazy_scan_prune "freeze the page" path. These newly revised comments are based on suggestions from Jeff Davis. In passing, remove nearby visibility_cutoff_xid comments left over from commit 6daeeb1f. Author: Peter Geoghegan Reviewed-By: Jeff Davis Discussion: https://postgr.es/m/ebc857107fe3edd422ef8a65191ca4a8da568b9b.camel@j-davis.com --- src/backend/access/heap/vacuumlazy.c | 20 +++++++------------- src/include/access/heapam.h | 23 ++++++++--------------- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 06fd15405f..21bd104f69 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1788,13 +1788,13 @@ retry: if (tuples_frozen == 0) { /* - * We're freezing all eligible tuples on the page, but have no - * freeze plans to execute. This is structured as a case where - * the page is nominally frozen so that we set pages all-frozen - * whenever no freeze plans need to be executed to make it safe. - * If this was handled via "no freeze" processing instead then - * VACUUM would senselessly waste certain opportunities to set - * pages all-frozen (not just all-visible) at no added cost. + * We have no freeze plans to execute, so there's no added cost + * from following the freeze path. That's why it was chosen. + * This is important in the case where the page only contains + * totally frozen tuples at this point (perhaps only following + * pruning). Such pages can be marked all-frozen in the VM by our + * caller, even though none of its tuples were newly frozen here + * (note that the "no freeze" path never sets pages all-frozen). * * We never increment the frozen_pages instrumentation counter * here, since it only counts pages with newly frozen tuples @@ -1859,12 +1859,6 @@ retry: if (!heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen)) Assert(false); - /* - * It's possible that we froze tuples and made the page's XID cutoff - * (for recovery conflict purposes) FrozenTransactionId. This is okay - * because visibility_cutoff_xid will be logged by our caller in a - * moment. - */ Assert(!TransactionIdIsValid(cutoff) || cutoff == prunestate->visibility_cutoff_xid); } diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 39c009dbf2..4a69b6dd04 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -145,16 +145,14 @@ typedef struct HeapPageFreeze /* * "Freeze" NewRelfrozenXid/NewRelminMxid trackers. * - * Trackers used when heap_freeze_execute_prepared freezes the page, and - * when page is "nominally frozen", which happens with pages where every - * call to heap_prepare_freeze_tuple produced no usable freeze plan. - * - * "Nominal freezing" enables vacuumlazy.c's approach of setting a page - * all-frozen in the visibility map when every tuple's 'totally_frozen' - * result is true. That always works in the same way, independent of the - * need to freeze tuples, and without complicating the general rule around - * 'totally_frozen' results (which is that 'totally_frozen' results are - * only to be trusted with a page that goes on to be frozen by caller). + * Trackers used when heap_freeze_execute_prepared freezes, or when there + * are zero freeze plans for a page. It is always valid for vacuumlazy.c + * to freeze any page, by definition. This even includes pages that have + * no tuples with storage to consider in the first place. That way the + * 'totally_frozen' results from heap_prepare_freeze_tuple can always be + * used in the same way, even when no freeze plans need to be executed to + * "freeze the page". Only the "freeze" path needs to consider the need + * to set pages all-frozen in the visibility map under this scheme. * * When we freeze a page, we generally freeze all XIDs < OldestXmin, only * leaving behind XIDs that are ineligible for freezing, if any. And so @@ -178,11 +176,6 @@ typedef struct HeapPageFreeze * VACUUM scans a page that isn't cleanup locked. Both code paths are * based on the same general idea (do less work for this page during the * ongoing VACUUM, at the cost of having to accept older final values). - * - * When vacuumlazy.c caller decides to do "no freeze" processing, it must - * not go on to set the page all-frozen (setting the page all-visible - * could still be okay). heap_prepare_freeze_tuple's 'totally_frozen' - * results can only be used on a page that also gets frozen as instructed. */ TransactionId NoFreezePageRelfrozenXid; MultiXactId NoFreezePageRelminMxid; -- 2.30.2