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 <[email protected]>
Reviewed-By: Jeff Davis <[email protected]>
Discussion: https://postgr.es/m/ebc857107fe3edd422ef8a65191ca4a8da568b9b[email protected]

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 06fd15405f755d08a784bde49b3cdeeecb9f0a1e..21bd104f69d4081596b9b0d47d865f4086207914 100644 (file)
--- 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 39c009dbf292320bcdcfccb63e9c7018dab59232..4a69b6dd0431908365047cf499b833186d6b07a1 100644 (file)
--- 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;