v11-0003-Don-t-force-aggressive-mode-for-DISABLE_PAGE_SKI.patch

From db98e5f02714bea3ce5422a68403b0a48dd280a7 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Thu, 17 Mar 2022 21:39:01 -0700
Subject: [PATCH v11 3/3] Don't force aggressive mode for
 DISABLE_PAGE_SKIPPING.

It seems more natural to just make this option about the visibility map,
not whether or not individual pages are processed using lazy_scan_prune
or lazy_scan_noprune.  The latter arguably doesn't really skip at all.

TODO Review implications for use of DISABLE_PAGE_SKIPPING in tests
changed by commits c2dc1a79 and fe246d1c11.  This probably won't revive
the issues addressed in those commits.

VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) is used for some of these tests
already, presumably because it was necessary to specify FREEZE to force
vacuum_freeze_min_age=0 to get stable results (in the individual cases
that use FREEZE too).
---
 src/include/commands/vacuum.h        |  2 +-
 src/backend/access/heap/vacuumlazy.c | 16 ++--------------
 doc/src/sgml/ref/vacuum.sgml         | 15 +++++----------
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index ead88edda..e0908012d 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -187,7 +187,7 @@ typedef struct VacAttrStats
 #define VACOPT_FULL 0x10		/* FULL (non-concurrent) vacuum */
 #define VACOPT_SKIP_LOCKED 0x20 /* skip if cannot get lock */
 #define VACOPT_PROCESS_TOAST 0x40	/* process the TOAST table, if any */
-#define VACOPT_DISABLE_PAGE_SKIPPING 0x80	/* don't skip any pages */
+#define VACOPT_DISABLE_PAGE_SKIPPING 0x80	/* don't skip any pages via VM */
 
 /*
  * Values used by index_cleanup and truncate params.
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 49653ae99..498c2f6ee 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -320,8 +320,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 	int			usecs;
 	double		read_rate,
 				write_rate;
-	bool		aggressive,
-				skipwithvm;
+	bool		aggressive;
 	bool		frozenxid_updated,
 				minmulti_updated;
 	BlockNumber orig_rel_pages;
@@ -372,17 +371,6 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 									   &OldestXmin, &OldestMxact,
 									   &FreezeLimit, &MultiXactCutoff);
 
-	skipwithvm = true;
-	if (params->options & VACOPT_DISABLE_PAGE_SKIPPING)
-	{
-		/*
-		 * Force aggressive mode, and disable skipping blocks using the
-		 * visibility map (even those set all-frozen)
-		 */
-		aggressive = true;
-		skipwithvm = false;
-	}
-
 	/*
 	 * Setup error traceback support for ereport() first.  The idea is to set
 	 * up an error context callback to display additional information on any
@@ -445,7 +433,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 	Assert(params->truncate != VACOPTVALUE_UNSPECIFIED &&
 		   params->truncate != VACOPTVALUE_AUTO);
 	vacrel->aggressive = aggressive;
-	vacrel->skipwithvm = skipwithvm;
+	vacrel->skipwithvm = (params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0;
 	vacrel->failsafe_active = false;
 	vacrel->consider_bypass_optimization = true;
 	vacrel->do_index_vacuuming = true;
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 3df32b58e..aab2d6c53 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -155,16 +155,11 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     <listitem>
      <para>
       Normally, <command>VACUUM</command> will skip pages based on the <link
-      linkend="vacuum-for-visibility-map">visibility map</link>.  Pages where
-      all tuples are known to be frozen can always be skipped, and those
-      where all tuples are known to be visible to all transactions may be
-      skipped except when performing an aggressive vacuum.  Furthermore,
-      except when performing an aggressive vacuum, some pages may be skipped
-      in order to avoid waiting for other sessions to finish using them.
-      This option disables all page-skipping behavior, and is intended to
-      be used only when the contents of the visibility map are
-      suspect, which should happen only if there is a hardware or software
-      issue causing database corruption.
+      linkend="vacuum-for-visibility-map">visibility map</link>.
+      This option disables that behavior, and is intended to be used
+      only when the contents of the visibility map are suspect, which
+      should happen only if there is a hardware or software issue
+      causing database corruption.
      </para>
     </listitem>
    </varlistentry>
-- 
2.30.2