v14-0002-Separate-vacuum-cost-variables-from-gucs.patch

From 7ee209bee5c096601ed3ad76ee4ab9ced340e3d3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 3 Apr 2023 11:22:18 -0400
Subject: [PATCH v14 2/4] Separate vacuum cost variables from gucs

Vacuum code run both by autovacuum workers and a backend doing
VACUUM/ANALYZE previously used VacuumCostLimit and VacuumCostDelay which
were the global variables for the gucs vacuum_cost_limit and
vacuum_cost_delay. Autovacuum workers needed to override these variables
with their own values, derived from autovacuum_vacuum_cost_limit and
autovacuum_vacuum_cost_delay and worker cost limit balancing logic. This
led to confusing code which, in some cases, both derived and set a new
value of VacuumCostLimit from VacuumCostLimit.

In preparation for refreshing these guc values more often, separate
these variables from the gucs themselves and add a function to update
the global variables using the gucs and existing logic.

Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com
---
 src/backend/commands/vacuum.c         | 16 ++++++++--
 src/backend/commands/vacuumparallel.c |  3 +-
 src/backend/postmaster/autovacuum.c   | 43 +++++++++++++--------------
 src/backend/utils/init/globals.c      |  2 --
 src/backend/utils/misc/guc_tables.c   |  4 +--
 src/include/commands/vacuum.h         |  7 +++++
 src/include/miscadmin.h               |  2 --
 src/include/postmaster/autovacuum.h   |  3 --
 8 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 9724fbce46..96df5e2920 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -71,6 +71,18 @@ int			vacuum_multixact_freeze_min_age;
 int			vacuum_multixact_freeze_table_age;
 int			vacuum_failsafe_age;
 int			vacuum_multixact_failsafe_age;
+double		vacuum_cost_delay;
+int			vacuum_cost_limit;
+
+/*
+ * Variables for cost-based vacuum delay. The defaults differ between
+ * autovacuum and vacuum. These should be overridden with the appropriate GUC
+ * value in vacuum code.
+ * TODO: should VacuumCostLimit and VacuumCostDelay be initialized to valid or
+ * invalid values?
+ */
+int			VacuumCostLimit = 0;
+double		VacuumCostDelay = -1;
 
 /*
  * VacuumFailsafeActive is a defined as a global so that we can determine
@@ -498,6 +510,7 @@ vacuum(List *relations, VacuumParams *params,
 	{
 		ListCell   *cur;
 
+		VacuumUpdateCosts();
 		in_vacuum = true;
 		VacuumCostActive = (VacuumCostDelay > 0);
 		VacuumCostBalance = 0;
@@ -2258,8 +2271,7 @@ vacuum_delay_point(void)
 
 		VacuumCostBalance = 0;
 
-		/* update balance values for workers */
-		AutoVacuumUpdateDelay();
+		VacuumUpdateCosts();
 
 		/* Might have gotten an interrupt while sleeping */
 		CHECK_FOR_INTERRUPTS();
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 563117a8f6..d346838cfc 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -995,8 +995,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
 												 false);
 
 	/* Set cost-based vacuum delay */
-	VacuumCostActive = (VacuumCostDelay > 0);
 	VacuumCostBalance = 0;
+	VacuumUpdateCosts();
+	VacuumCostActive = (VacuumCostDelay > 0);
 	VacuumPageHit = 0;
 	VacuumPageMiss = 0;
 	VacuumPageDirty = 0;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 585d28148c..e0c568fdaf 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -1774,17 +1774,28 @@ FreeWorkerInfo(int code, Datum arg)
 }
 
 /*
- * Update the cost-based delay parameters, so that multiple workers consume
- * each a fraction of the total available I/O.
+ * Update vacuum cost-based delay-related parameters for autovacuum workers and
+ * backends executing VACUUM or ANALYZE using the value of relevant gucs and
+ * global state. This must be called during setup for vacuum and after every
+ * config reload to ensure up-to-date values.
  */
 void
-AutoVacuumUpdateDelay(void)
+VacuumUpdateCosts(void)
 {
-	if (MyWorkerInfo)
+	if (am_autovacuum_launcher)
+		return;
+
+	if (am_autovacuum_worker)
 	{
 		VacuumCostDelay = MyWorkerInfo->wi_cost_delay;
 		VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
 	}
+	else
+	{
+		/* Must be explicit VACUUM or ANALYZE */
+		VacuumCostLimit = vacuum_cost_limit;
+		VacuumCostDelay = vacuum_cost_delay;
+	}
 }
 
 /*
@@ -1805,9 +1816,9 @@ autovac_balance_cost(void)
 	 * zero is not a valid value.
 	 */
 	int			vac_cost_limit = (autovacuum_vac_cost_limit > 0 ?
-								  autovacuum_vac_cost_limit : VacuumCostLimit);
+								  autovacuum_vac_cost_limit : vacuum_cost_limit);
 	double		vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ?
-								  autovacuum_vac_cost_delay : VacuumCostDelay);
+								  autovacuum_vac_cost_delay : vacuum_cost_delay);
 	double		cost_total;
 	double		cost_avail;
 	dlist_iter	iter;
@@ -2312,8 +2323,6 @@ do_autovacuum(void)
 		autovac_table *tab;
 		bool		isshared;
 		bool		skipit;
-		double		stdVacuumCostDelay;
-		int			stdVacuumCostLimit;
 		dlist_iter	iter;
 
 		CHECK_FOR_INTERRUPTS();
@@ -2416,14 +2425,6 @@ do_autovacuum(void)
 			continue;
 		}
 
-		/*
-		 * Remember the prevailing values of the vacuum cost GUCs.  We have to
-		 * restore these at the bottom of the loop, else we'll compute wrong
-		 * values in the next iteration of autovac_balance_cost().
-		 */
-		stdVacuumCostDelay = VacuumCostDelay;
-		stdVacuumCostLimit = VacuumCostLimit;
-
 		/* Must hold AutovacuumLock while mucking with cost balance info */
 		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
 
@@ -2437,7 +2438,7 @@ do_autovacuum(void)
 		autovac_balance_cost();
 
 		/* set the active cost parameters from the result of that */
-		AutoVacuumUpdateDelay();
+		VacuumUpdateCosts();
 
 		/* done */
 		LWLockRelease(AutovacuumLock);
@@ -2534,10 +2535,6 @@ deleted:
 		MyWorkerInfo->wi_tableoid = InvalidOid;
 		MyWorkerInfo->wi_sharedrel = false;
 		LWLockRelease(AutovacuumScheduleLock);
-
-		/* restore vacuum cost GUCs for the next iteration */
-		VacuumCostDelay = stdVacuumCostDelay;
-		VacuumCostLimit = stdVacuumCostLimit;
 	}
 
 	/*
@@ -2820,14 +2817,14 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 			? avopts->vacuum_cost_delay
 			: (autovacuum_vac_cost_delay >= 0)
 			? autovacuum_vac_cost_delay
-			: VacuumCostDelay;
+			: vacuum_cost_delay;
 
 		/* 0 or -1 in autovac setting means use plain vacuum_cost_limit */
 		vac_cost_limit = (avopts && avopts->vacuum_cost_limit > 0)
 			? avopts->vacuum_cost_limit
 			: (autovacuum_vac_cost_limit > 0)
 			? autovacuum_vac_cost_limit
-			: VacuumCostLimit;
+			: vacuum_cost_limit;
 
 		/* -1 in autovac setting means use log_autovacuum_min_duration */
 		log_min_duration = (avopts && avopts->log_min_duration >= 0)
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..8e5b065e8f 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -142,8 +142,6 @@ int			MaxBackends = 0;
 int			VacuumCostPageHit = 1;	/* GUC parameters for vacuum */
 int			VacuumCostPageMiss = 2;
 int			VacuumCostPageDirty = 20;
-int			VacuumCostLimit = 200;
-double		VacuumCostDelay = 0;
 
 int64		VacuumPageHit = 0;
 int64		VacuumPageMiss = 0;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8062589efd..77db1a146c 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2409,7 +2409,7 @@ struct config_int ConfigureNamesInt[] =
 			gettext_noop("Vacuum cost amount available before napping."),
 			NULL
 		},
-		&VacuumCostLimit,
+		&vacuum_cost_limit,
 		200, 1, 10000,
 		NULL, NULL, NULL
 	},
@@ -3701,7 +3701,7 @@ struct config_real ConfigureNamesReal[] =
 			NULL,
 			GUC_UNIT_MS
 		},
-		&VacuumCostDelay,
+		&vacuum_cost_delay,
 		0, 0, 100,
 		NULL, NULL, NULL
 	},
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 7b8ee21788..a62dd2e781 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -300,6 +300,8 @@ extern PGDLLIMPORT int vacuum_multixact_freeze_min_age;
 extern PGDLLIMPORT int vacuum_multixact_freeze_table_age;
 extern PGDLLIMPORT int vacuum_failsafe_age;
 extern PGDLLIMPORT int vacuum_multixact_failsafe_age;
+extern PGDLLIMPORT double vacuum_cost_delay;
+extern PGDLLIMPORT int vacuum_cost_limit;
 
 /* Variables for cost-based parallel vacuum */
 extern PGDLLIMPORT pg_atomic_uint32 *VacuumSharedCostBalance;
@@ -307,6 +309,8 @@ extern PGDLLIMPORT pg_atomic_uint32 *VacuumActiveNWorkers;
 extern PGDLLIMPORT int VacuumCostBalanceLocal;
 
 extern bool VacuumFailsafeActive;
+extern int	VacuumCostLimit;
+extern double VacuumCostDelay;
 
 /* in commands/vacuum.c */
 extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
@@ -346,6 +350,9 @@ extern IndexBulkDeleteResult *vac_cleanup_one_index(IndexVacuumInfo *ivinfo,
 													IndexBulkDeleteResult *istat);
 extern Size vac_max_items_to_alloc_size(int max_items);
 
+/* In postmaster/autovacuum.c */
+extern void VacuumUpdateCosts(void);
+
 /* in commands/vacuumparallel.c */
 extern ParallelVacuumState *parallel_vacuum_init(Relation rel, Relation *indrels,
 												 int nindexes, int nrequested_workers,
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1..66db1b2c69 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -266,8 +266,6 @@ extern PGDLLIMPORT int max_parallel_maintenance_workers;
 extern PGDLLIMPORT int VacuumCostPageHit;
 extern PGDLLIMPORT int VacuumCostPageMiss;
 extern PGDLLIMPORT int VacuumCostPageDirty;
-extern PGDLLIMPORT int VacuumCostLimit;
-extern PGDLLIMPORT double VacuumCostDelay;
 
 extern PGDLLIMPORT int64 VacuumPageHit;
 extern PGDLLIMPORT int64 VacuumPageMiss;
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index c140371b51..65afd1ea1e 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -63,9 +63,6 @@ extern int	StartAutoVacWorker(void);
 /* called from postmaster when a worker could not be forked */
 extern void AutoVacWorkerFailed(void);
 
-/* autovacuum cost-delay balancer */
-extern void AutoVacuumUpdateDelay(void);
-
 #ifdef EXEC_BACKEND
 extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
 extern void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn();
-- 
2.37.2