v1-0004-add-vacuum-option-to-specify-nbuffers.patch

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

---
 contrib/amcheck/verify_heapam.c       |  2 +-
 contrib/amcheck/verify_nbtree.c       |  2 +-
 contrib/bloom/blscan.c                |  2 +-
 contrib/pg_visibility/pg_visibility.c |  4 +-
 contrib/pgstattuple/pgstatapprox.c    |  2 +-
 contrib/pgstattuple/pgstatindex.c     |  4 +-
 contrib/pgstattuple/pgstattuple.c     |  2 +-
 src/backend/access/heap/heapam.c      |  4 +-
 src/backend/commands/dbcommands.c     |  2 +-
 src/backend/commands/vacuum.c         | 50 ++++++++++++++++++++++-
 src/backend/commands/vacuumparallel.c |  8 +++-
 src/backend/postmaster/autovacuum.c   |  5 ++-
 src/backend/storage/buffer/bufmgr.c   |  4 +-
 src/backend/storage/buffer/freelist.c | 58 ++++++++++++++++++++++++---
 src/include/commands/vacuum.h         |  1 +
 src/include/storage/bufmgr.h          |  2 +-
 16 files changed, 127 insertions(+), 25 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 4fcfd6df72..e3aa89975d 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -330,7 +330,7 @@ verify_heapam(PG_FUNCTION_ARGS)
 		PG_RETURN_NULL();
 	}
 
-	ctx.bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	ctx.bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 	ctx.buffer = InvalidBuffer;
 	ctx.page = NULL;
 
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 257cff671b..b0552a94ef 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -547,7 +547,7 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace,
 	state->targetcontext = AllocSetContextCreate(CurrentMemoryContext,
 												 "amcheck context",
 												 ALLOCSET_DEFAULT_SIZES);
-	state->checkstrategy = GetAccessStrategy(BAS_BULKREAD);
+	state->checkstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	/* Get true root block from meta-page */
 	metapage = palloc_btree_page(state, BTREE_METAPAGE);
diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index 6cc7d07164..10e3086e39 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -119,7 +119,7 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	 * We're going to read the whole index. This is why we use appropriate
 	 * buffer access strategy.
 	 */
-	bas = GetAccessStrategy(BAS_BULKREAD);
+	bas = GetAccessStrategy(BAS_BULKREAD, -1);
 	npages = RelationGetNumberOfBlocks(scan->indexRelation);
 
 	for (blkno = BLOOM_HEAD_BLKNO; blkno < npages; blkno++)
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 2a4acfd1ee..47a113db67 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -476,7 +476,7 @@ collect_visibility_data(Oid relid, bool include_pd)
 	vbits	   *info;
 	BlockNumber blkno;
 	Buffer		vmbuffer = InvalidBuffer;
-	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	rel = relation_open(relid, AccessShareLock);
 
@@ -554,7 +554,7 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
 	corrupt_items *items;
 	BlockNumber blkno;
 	Buffer		vmbuffer = InvalidBuffer;
-	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 	TransactionId OldestXmin = InvalidTransactionId;
 
 	rel = relation_open(relid, AccessShareLock);
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index f601dc6121..e71a468988 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -72,7 +72,7 @@ statapprox_heap(Relation rel, output_type *stat)
 	TransactionId OldestXmin;
 
 	OldestXmin = GetOldestNonRemovableTransactionId(rel);
-	bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	nblocks = RelationGetNumberOfBlocks(rel);
 	scanned = 0;
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index d69ac1c93d..53c6e99bfa 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -219,7 +219,7 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
 	BlockNumber nblocks;
 	BlockNumber blkno;
 	BTIndexStat indexStat;
-	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	if (!IS_INDEX(rel) || !IS_BTREE(rel))
 		ereport(ERROR,
@@ -612,7 +612,7 @@ pgstathashindex(PG_FUNCTION_ARGS)
 	nblocks = RelationGetNumberOfBlocks(rel);
 
 	/* prepare access strategy for this index */
-	bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	/* Start from blkno 1 as 0th block is metapage */
 	for (blkno = 1; blkno < nblocks; blkno++)
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 93b7834b77..587d79bb38 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -518,7 +518,7 @@ pgstat_index(Relation rel, BlockNumber start, pgstat_page pagefn,
 	pgstattuple_type stat = {0};
 
 	/* prepare access strategy for this index */
-	bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	blkno = start;
 	for (;;)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7eb79cee58..6a3efe3d2d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -280,7 +280,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
 	{
 		/* During a rescan, keep the previous strategy object. */
 		if (scan->rs_strategy == NULL)
-			scan->rs_strategy = GetAccessStrategy(BAS_BULKREAD);
+			scan->rs_strategy = GetAccessStrategy(BAS_BULKREAD, -1);
 	}
 	else
 	{
@@ -1772,7 +1772,7 @@ GetBulkInsertState(void)
 	BulkInsertState bistate;
 
 	bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
-	bistate->strategy = GetAccessStrategy(BAS_BULKWRITE);
+	bistate->strategy = GetAccessStrategy(BAS_BULKWRITE, -1);
 	bistate->current_buf = InvalidBuffer;
 	return bistate;
 }
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index a0259cc593..51f3d64de7 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -280,7 +280,7 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char *srcpath)
 	smgrclose(smgr);
 
 	/* Use a buffer access strategy since this is a bulk read operation. */
-	bstrategy = GetAccessStrategy(BAS_BULKREAD);
+	bstrategy = GetAccessStrategy(BAS_BULKREAD, -1);
 
 	/*
 	 * As explained in the function header comments, we need a snapshot that
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 78980cf19c..b3fded878b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -126,6 +126,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	/* By default parallel vacuum is enabled */
 	params.nworkers = 0;
 
+	/* default value of buffers buffer access strategy */
+	params.buffers = -1;
+
+
 	/* Parse options list */
 	foreach(lc, vacstmt->options)
 	{
@@ -207,6 +211,45 @@ 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)
+		{
+			// TODO: default_ring_size calculated here and GetAccessStrategy() == bad
+			int vac_buffers;
+			int default_ring_size = 256 * 1024 / BLCKSZ;
+			int max_ring_size = NBuffers / 8;
+
+			if (opt->arg == NULL)
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("buffer_usage_limit option requires an integer value"),
+						 parser_errposition(pstate, opt->location)));
+			}
+
+			vac_buffers = defGetInt64(opt);
+
+			/* out-of-bounds cases */
+			if (vac_buffers < -1 || vac_buffers > max_ring_size)
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+							errmsg("buffer_usage_limit for a vacuum must be between -1 and %d",
+									max_ring_size),
+							parser_errposition(pstate, opt->location)));
+			}
+
+			if (vac_buffers == -1)
+				vac_buffers = default_ring_size;
+			else if (vac_buffers < default_ring_size)
+			{
+				elog(WARNING, "buffer_usage_limit %d is below the minimum buffer_usage_limit of %d. setting it to %d",
+						vac_buffers, default_ring_size, default_ring_size);
+
+				vac_buffers = default_ring_size;
+			}
+
+			params.buffers = vac_buffers;
+		}
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -393,8 +436,7 @@ vacuum(List *relations, VacuumParams *params,
 	if (bstrategy == NULL)
 	{
 		MemoryContext old_context = MemoryContextSwitchTo(vac_context);
-
-		bstrategy = GetAccessStrategy(BAS_VACUUM);
+		bstrategy = GetAccessStrategy(BAS_VACUUM, params->buffers);
 		MemoryContextSwitchTo(old_context);
 	}
 
@@ -2066,7 +2108,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs,
 		cluster_rel(relid, InvalidOid, &cluster_params);
 	}
 	else
+	{
+		if (params->buffers == 0)
+			bstrategy = NULL;
 		table_relation_vacuum(rel, params, bstrategy);
+	}
 
 	/* Roll back any GUC changes executed by index functions */
 	AtEOXact_GUC(false, save_nestlevel);
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index bcd40c80a1..2c8380bd2e 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1012,8 +1012,12 @@ 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 */
-	pvs.bstrategy = GetAccessStrategy(BAS_VACUUM);
+	/*
+	 * 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, -1);
 
 	/* Setup error traceback support for ereport() */
 	errcallback.callback = parallel_vacuum_error_callback;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 673ff99767..3d88d72973 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2291,8 +2291,11 @@ 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);
+	bstrategy = GetAccessStrategy(BAS_VACUUM, -1);
 
 	/*
 	 * create a memory context to act as fake PortalContext, so that the
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 98904a7c05..62ef088306 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3818,8 +3818,8 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator,
 			   buf.data, true);
 
 	/* This is a bulk operation, so use buffer access strategies. */
-	bstrategy_src = GetAccessStrategy(BAS_BULKREAD);
-	bstrategy_dst = GetAccessStrategy(BAS_BULKWRITE);
+	bstrategy_src = GetAccessStrategy(BAS_BULKREAD, -1);
+	bstrategy_dst = GetAccessStrategy(BAS_BULKWRITE, -1);
 
 	/* Iterate over each block of the source relation file. */
 	for (blkno = 0; blkno < nblocks; blkno++)
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index c690d5f15f..24d21cc4f1 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -531,23 +531,61 @@ StrategyInitialize(bool init)
  * ----------------------------------------------------------------
  */
 
+static const char *
+btype_get_name(BufferAccessStrategyType btype)
+{
+	switch (btype)
+	{
+		case BAS_NORMAL:
+			return "normal";
+		case BAS_BULKREAD:
+			return "bulkread";
+		case BAS_BULKWRITE:
+			return "bulkwrite";
+		case BAS_VACUUM:
+			return "vacuum";
+	}
+
+	elog(ERROR, "unrecognized BufferAccessStrategy: %d", btype);
+	pg_unreachable();
+}
+
+
 
 /*
  * GetAccessStrategy -- create a BufferAccessStrategy object
  *
  * The object is allocated in the current memory context.
  */
+// TODO: make a macro or something for nbuffers = -1
 BufferAccessStrategy
-GetAccessStrategy(BufferAccessStrategyType btype)
+GetAccessStrategy(BufferAccessStrategyType btype, int buffers)
 {
 	BufferAccessStrategy strategy;
 	int			ring_size;
+	const char *strategy_name = btype_get_name(btype);
+
+	if (btype != BAS_VACUUM)
+	{
+		if (buffers == 0)
+			elog(ERROR, "Use of shared buffers unsupported for buffer access strategy: %s. nbuffers must be -1.",
+					strategy_name);
+
+		if (buffers > 0)
+			elog(ERROR, "Specification of ring size in buffers unsupported for buffer access strategy: %s. nbuffers must be -1.",
+					strategy_name);
+	}
+
+	// TODO: DEBUG logging message for dev?
+	if (buffers == 0)
+		btype = BAS_NORMAL;
 
 	/*
 	 * Select ring size to use.  See buffer/README for rationales.
 	 *
 	 * Note: if you change the ring size for BAS_BULKREAD, see also
 	 * SYNC_SCAN_REPORT_INTERVAL in access/heap/syncscan.c.
+	 * TODO: update README
 	 */
 	switch (btype)
 	{
@@ -562,16 +600,26 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			ring_size = 16 * 1024 * 1024 / BLCKSZ;
 			break;
 		case BAS_VACUUM:
-			ring_size = 256 * 1024 / BLCKSZ;
-			break;
+		{
+			int default_ring_size = 256 * 1024 / BLCKSZ;
 
+			if (buffers == -1)
+				ring_size = default_ring_size;
+			else
+				ring_size = Max(buffers, default_ring_size);
+			// TODO: log if we end up setting ring_size bigger than nbuffers because it would have been smaller than the default
+			break;
+		}
+		/* unreachable as long as we get the string representation of the type above */
 		default:
 			elog(ERROR, "unrecognized buffer access strategy: %d",
-				 (int) btype);
-			return NULL;		/* keep compiler quiet */
+					(int) btype);
+
+		pg_unreachable();
 	}
 
 	/* Make sure ring isn't an undue fraction of shared buffers */
+	// TODO: log message that tells you if your specified number of buffers is clamped
 	ring_size = Min(NBuffers / 8, ring_size);
 
 	/* Allocate the object and initialize all elements to zeroes */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 689dbb7702..341f1e2525 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -235,6 +235,7 @@ typedef struct VacuumParams
 	 * disabled.
 	 */
 	int			nworkers;
+	int buffers;
 } VacuumParams;
 
 /*
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index b8a18b8081..2157200973 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -195,7 +195,7 @@ extern Size BufferShmemSize(void);
 extern void AtProcExit_LocalBuffers(void);
 
 /* in freelist.c */
-extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype);
+extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype, int nbuffers);
 extern void FreeAccessStrategy(BufferAccessStrategy strategy);
 
 
-- 
2.37.2