From d9a0183b98601e4299b8b12a4ae06e9aaf5c1bfe Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 22 Feb 2023 15:28:34 -0500
Subject: [PATCH v2 3/3] add vacuum option to specify nbuffers and guc

---
 src/backend/commands/vacuum.c                 | 51 ++++++++++++++++++-
 src/backend/commands/vacuumparallel.c         |  6 ++-
 src/backend/postmaster/autovacuum.c           | 15 +++++-
 src/backend/storage/buffer/README             |  2 +
 src/backend/storage/buffer/freelist.c         | 40 +++++++++++++++
 src/backend/utils/init/globals.c              |  2 +
 src/backend/utils/misc/guc_tables.c           | 11 ++++
 src/backend/utils/misc/postgresql.conf.sample |  3 ++
 src/include/commands/vacuum.h                 |  1 +
 src/include/miscadmin.h                       |  1 +
 src/include/storage/bufmgr.h                  |  5 ++
 11 files changed, 134 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index c62be52e3b..dc51d69821 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -127,6 +127,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	/* By default parallel vacuum is enabled */
 	params.nworkers = 0;
 
+	/* by default use buffer access strategy with default size */
+	params.ring_size = -1;
+
 	/* Parse options list */
 	foreach(lc, vacstmt->options)
 	{
@@ -210,6 +213,43 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			skip_database_stats = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "only_database_stats") == 0)
 			only_database_stats = defGetBoolean(opt);
+		else if (strcmp(opt->defname, "buffer_usage_limit") == 0)
+		{
+			char *vac_buffer_size;
+			int result;
+
+			if (opt->arg == NULL)
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("buffer_usage_limit option requires a valid value"),
+						 parser_errposition(pstate, opt->location)));
+			}
+
+			vac_buffer_size = defGetString(opt);
+
+			if (!parse_int(vac_buffer_size, &result, GUC_UNIT_KB, NULL))
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+							errmsg("buffer_usage_limit for a vacuum must be between -1 and %d. %s is invalid.",
+									MAX_BAS_RING_SIZE_KB, vac_buffer_size),
+							parser_errposition(pstate, opt->location)));
+			}
+
+			/* check for out-of-bounds */
+			if (result < -1 || result > MAX_BAS_RING_SIZE_KB)
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+							errmsg("buffer_usage_limit for a vacuum must be between -1 and %d",
+									MAX_BAS_RING_SIZE_KB),
+							parser_errposition(pstate, opt->location)));
+			}
+
+			params.ring_size = result;
+
+		}
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -399,7 +439,16 @@ vacuum(List *relations, VacuumParams *params,
 	{
 		MemoryContext old_context = MemoryContextSwitchTo(vac_context);
 
-		bstrategy = GetAccessStrategy(BAS_VACUUM);
+		if (params->ring_size == -1)
+		{
+			if (buffer_usage_limit == -1)
+				bstrategy = GetAccessStrategy(BAS_VACUUM);
+			else
+				bstrategy = GetAccessStrategyExt(BAS_VACUUM, buffer_usage_limit);
+		}
+		else
+			bstrategy = GetAccessStrategyExt(BAS_VACUUM, params->ring_size);
+
 		MemoryContextSwitchTo(old_context);
 	}
 
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index bcd40c80a1..16742f6612 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1012,7 +1012,11 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
 	pvs.indname = NULL;
 	pvs.status = PARALLEL_INDVAC_STATUS_INITIAL;
 
-	/* Each parallel VACUUM worker gets its own access strategy */
+	/*
+	 * Each parallel VACUUM worker gets its own access strategy
+	 * For now, use the default buffer access strategy ring size.
+	 * TODO: should this work differently even though they are only for indexes
+	 */
 	pvs.bstrategy = GetAccessStrategy(BAS_VACUUM);
 
 	/* Setup error traceback support for ereport() */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c0e2e00a7e..ffd9308952 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2291,8 +2291,14 @@ do_autovacuum(void)
 	 * Create a buffer access strategy object for VACUUM to use.  We want to
 	 * use the same one across all the vacuum operations we perform, since the
 	 * point is for VACUUM not to blow out the shared cache.
+	 * If we later enter failsafe mode, we will cease use of the
+	 * BufferAccessStrategy. Either way, we clean up the BufferAccessStrategy
+	 * object at the end of this function.
 	 */
-	bstrategy = GetAccessStrategy(BAS_VACUUM);
+	if (buffer_usage_limit == -1)
+		bstrategy = GetAccessStrategy(BAS_VACUUM);
+	else
+		bstrategy = GetAccessStrategyExt(BAS_VACUUM, buffer_usage_limit);
 
 	/*
 	 * create a memory context to act as fake PortalContext, so that the
@@ -2881,6 +2887,13 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;
 		tab->at_params.is_wraparound = wraparound;
 		tab->at_params.log_min_duration = log_min_duration;
+
+		// TODO: should this be 0 so that we are sure that vacuum() never
+		// allocates a new bstrategy for us, even if we pass in NULL for that
+		// parameter? maybe could change how failsafe NULLs out bstrategy if
+		// so?
+		tab->at_params.ring_size = buffer_usage_limit;
+
 		tab->at_vacuum_cost_limit = vac_cost_limit;
 		tab->at_vacuum_cost_delay = vac_cost_delay;
 		tab->at_relname = NULL;
diff --git a/src/backend/storage/buffer/README b/src/backend/storage/buffer/README
index a775276ff2..bf166bbdf1 100644
--- a/src/backend/storage/buffer/README
+++ b/src/backend/storage/buffer/README
@@ -245,6 +245,8 @@ for WAL flushes.  While it's okay for a background vacuum to be slowed by
 doing its own WAL flushing, we'd prefer that COPY not be subject to that,
 so we let it use up a bit more of the buffer arena.
 
+TODO: update with info about new option to control the size
+
 
 Background Writer's Processing
 ------------------------------
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index c690d5f15f..a7e29f85ac 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -22,6 +22,7 @@
 #include "storage/proc.h"
 
 #define INT_ACCESS_ONCE(var)	((int)(*((volatile int *)&(var))))
+#define bufsize_limit_to_nbuffers(bufsize) (bufsize * 1024 / BLCKSZ)
 
 
 /*
@@ -586,6 +587,45 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 	return strategy;
 }
 
+
+BufferAccessStrategy
+GetAccessStrategyExt(BufferAccessStrategyType btype, int ring_size)
+{
+	BufferAccessStrategy strategy;
+	int nbuffers;
+
+	/* Default nbuffers should have resulted in calling GetAccessStrategy() */
+	// TODO: this being called ring_size and also nbuffers being called
+	// ring_size in GetAccessStrategy is confusing. Rename the member of
+	// BufferAccessStrategy
+	Assert(ring_size != -1);
+
+	if (ring_size == 0)
+		return NULL;
+
+	Assert(ring_size < MAX_BAS_RING_SIZE_KB);
+
+	// TODO: warn about clamping?
+	if (ring_size < MIN_BAS_RING_SIZE_KB)
+		nbuffers = bufsize_limit_to_nbuffers(MIN_BAS_RING_SIZE_KB);
+	else
+		nbuffers = bufsize_limit_to_nbuffers(ring_size);
+
+	// TODO: make this smaller?
+	nbuffers = Min(NBuffers / 8, nbuffers);
+
+	/* Allocate the object and initialize all elements to zeroes */
+	strategy = (BufferAccessStrategy)
+		palloc0(offsetof(BufferAccessStrategyData, buffers) +
+				nbuffers * sizeof(Buffer));
+
+	/* Set field that didn't start out zero */
+	strategy->btype = btype;
+	strategy->ring_size = nbuffers;
+
+	return strategy;
+}
+
 /*
  * FreeAccessStrategy -- release a BufferAccessStrategy object
  *
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..5167ecddcc 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -139,6 +139,8 @@ int			max_worker_processes = 8;
 int			max_parallel_workers = 8;
 int			MaxBackends = 0;
 
+int			buffer_usage_limit = -1;
+
 int			VacuumCostPageHit = 1;	/* GUC parameters for vacuum */
 int			VacuumCostPageMiss = 2;
 int			VacuumCostPageDirty = 20;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1c0583fe26..55d4965d62 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2206,6 +2206,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"buffer_usage_limit", PGC_USERSET, RESOURCES_MEM,
+			gettext_noop("Sets the buffer pool size for operations employing a buffer access strategy."),
+			NULL,
+			GUC_UNIT_KB
+		},
+		&buffer_usage_limit,
+		-1, -1, MAX_BAS_RING_SIZE_KB,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"shared_memory_size", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the size of the server's main shared memory area (rounded up to the nearest MB)."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d06074b86f..d5a8d24621 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -156,6 +156,9 @@
 					#   mmap
 					# (change requires restart)
 #min_dynamic_shared_memory = 0MB	# (change requires restart)
+#buffer_usage_limit = -1 # size of buffer access strategy ring. -1 to use default,
+				# 0 to disable buffer access strategy and use shared buffers
+				# > 0 to specify size
 
 # - Disk -
 
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index bdfd96cfec..06bf66f4f4 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -236,6 +236,7 @@ typedef struct VacuumParams
 	 * disabled.
 	 */
 	int			nworkers;
+	int ring_size;
 } VacuumParams;
 
 /*
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1..7d7d91b1a4 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -262,6 +262,7 @@ extern PGDLLIMPORT int work_mem;
 extern PGDLLIMPORT double hash_mem_multiplier;
 extern PGDLLIMPORT int maintenance_work_mem;
 extern PGDLLIMPORT int max_parallel_maintenance_workers;
+extern PGDLLIMPORT int buffer_usage_limit;
 
 extern PGDLLIMPORT int VacuumCostPageHit;
 extern PGDLLIMPORT int VacuumCostPageMiss;
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index b8a18b8081..32148ba580 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -101,6 +101,9 @@ extern PGDLLIMPORT int32 *LocalRefCount;
 /* upper limit for effective_io_concurrency */
 #define MAX_IO_CONCURRENCY 1000
 
+#define MAX_BAS_RING_SIZE_KB (16 * 1024 * 1024)
+#define MIN_BAS_RING_SIZE_KB 128
+
 /* special block number for ReadBuffer() */
 #define P_NEW	InvalidBlockNumber	/* grow the file to get a new page */
 
@@ -196,6 +199,8 @@ extern void AtProcExit_LocalBuffers(void);
 
 /* in freelist.c */
 extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype);
+
+extern BufferAccessStrategy GetAccessStrategyExt(BufferAccessStrategyType btype, int nbuffers);
 extern void FreeAccessStrategy(BufferAccessStrategy strategy);
 
 
-- 
2.37.2

