On Wed, Apr 5, 2023 at 9:15 PM David Rowley <dgrowle...@gmail.com> wrote:
>
> On Thu, 6 Apr 2023 at 12:42, Melanie Plageman <melanieplage...@gmail.com> 
> wrote:
> > Attached is a v12 of the whole vacuum_buffer_usage_limit patch set which
> > includes a commit to fix the bug in master and a commit to move relevant
> > code from vacuum() up into ExecVacuum().
>
> I'm still playing catch up to the moving of the pre-checks from
> vacuum() to ExecVacuum().  I'm now wondering...
>
> Is it intended that VACUUM t1,t2; now share the same strategy?
> Currently, in master, we'll allocate a new strategy for t2 after
> vacuuming t1. Does this not mean we'll now leave fewer t1 pages in
> shared_buffers because the reuse of the strategy will force them out
> with t2 pages?  I understand there's nothing particularly invalid
> about that, but it is a change in behaviour that the patch seems to be
> making with very little consideration as to if it's better or worse.

I'm pretty sure that in master we also reuse the strategy since we make
it above this loop in vacuum() (and pass it in)

        /*
        * Loop to process each selected relation.
        */
        foreach(cur, relations)
        {
            VacuumRelation *vrel = lfirst_node(VacuumRelation, cur);
            if (params->options & VACOPT_VACUUM)
            {
                if (!vacuum_rel(vrel->oid, vrel->relation, params,
false, bstrategy))
                    continue;
            }

On Wed, Apr 5, 2023 at 8:41 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
>
> On Wed, Apr 5, 2023 at 6:55 PM Melanie Plageman
> <melanieplage...@gmail.com> wrote:
> >
> > On Wed, Apr 5, 2023 at 5:15 PM Andres Freund <and...@anarazel.de> wrote:
> > >
> > > Hi,
> > >
> > > On 2023-04-05 16:17:20 -0400, Melanie Plageman wrote:
> > > > On Wed, Apr 5, 2023 at 1:05 PM Andres Freund <and...@anarazel.de> wrote:
> > > > > Perhaps the best solution for autovac vs interactive vacuum issue 
> > > > > would be to
> > > > > move the allocation of the bstrategy to ExecVacuum()?
> > > >
> > > > So, I started looking into allocating the bstrategy in ExecVacuum().
> > > >
> > > > While doing so, I was trying to understand if the "sanity checking" in
> > > > vacuum() could possibly apply to autovacuum, and I don't really see how.
> > > >
> > > > AFAICT, autovacuum does not ever set VACOPT_DISABLE_PAGE_SKIPPING or
> > > > VACOPT_FULL or VACOPT_ONLY_DATABASE_STATS.
> > > >
> > > > We could move those sanity checks up into ExecVacuum().
> > >
> > > Would make sense.
> > >
> > > ISTM that eventually most of what currently happens in vacuum() should be 
> > > in
> > > ExecVacuum(). There's a lot of stuff that can't happen for autovacuum. So 
> > > it
> > > just seems to make more sense to move those parts to ExecVacuum().
> >
> > I've done that in the attached wip patch. It is perhaps too much of a
> > change, I dunno.
> >
> > > > I also noticed that we make the vac_context in vacuum() which says it is
> > > > for "cross-transaction storage". We use it for the buffer access
> > > > strategy and for the newrels relation list created in vacuum(). Then we
> > > > delete it at the end of vacuum().
> > >
> > > > Autovacuum workers already make a similar kind of memory context called
> > > > AutovacMemCxt in do_autovacuum() which the comment says is for the list
> > > > of relations to vacuum/analyze across transactions.
> > >
> > > AutovacMemCxt seems to be a bit longer lived / cover more than the context
> > > created in vacuum(). It's where all the hash tables etc live that
> > > do_autovacuum() uses to determine what to vacuum.
> > >
> > > Note that do_autovacuum() also creates:
> > >
> > >         /*
> > >          * create a memory context to act as fake PortalContext, so that 
> > > the
> > >          * contexts created in the vacuum code are cleaned up for each 
> > > table.
> > >          */
> > >         PortalContext = AllocSetContextCreate(AutovacMemCxt,
> > >                                                                           
> > >         "Autovacuum Portal",
> > >                                                                           
> > >         ALLOCSET_DEFAULT_SIZES);
> > >
> > > which is then what vacuum() creates the "Vacuum" context in.
> >
> > Yea, I realized that when writing the patch.
> >
> > > > What if we made ExecVacuum() make its own memory context and both it and
> > > > do_autovacuum() pass that memory context (along with the buffer access
> > > > strategy they make) to vacuum(), which then uses the memory context in
> > > > the same way it does now?
> > >
> > > Maybe? It's not clear to me why it'd be a win.
> >
> > Less that it is a win and more that we need access to that memory
> > context when allocating the buffer access strategy, so we would have had
> > to make it in ExecVacuum(). And if we have already made it, we would
> > need to pass it in to vacuum() for it to use.
> >
> > > > It simplifies the buffer usage limit patchset and also seems a bit more
> > > > clear than what is there now?
> > >
> > > I don't really see what it'd make simpler? The context in vacuum() is 
> > > used for
> > > just that vacuum - we couldn't just use AutovacMemCxt, as that'd live much
> > > longer (for all the tables a autovac worker processes).
> >
> > Autovacuum already made the BufferAccessStrategy in the AutovacMemCxt,
> > so this is the same behavior. I simply made autovacuum_do_vac_analyze()
> > make the per table vacuum memory context and pass that to vacuum(). So
> > we have the same amount of memory context granularity as before.
> >
> > Attached patchset has some kind of isolation test failure due to a hard
> > deadlock that I haven't figured out yet. I thought it was something with
> > the "in_vacuum" static variable and having VACUUM or ANALYZE called when
> > already in VACUUM or ANALYZE, but that variable is the same as in
> > master.
> >
> > I've mostly shared it because I want to know if this approach is worth
> > pursuing or not.
>
> Figured out how to fix the issue, though I'm not sure I understand how
> the issue can occur.
> use_own_xacts seems like it will always be true for autovacuum when it
> calls vacuum() and ExecVacuum() only calls vacuum() once, so I thought
> that I could make use_own_xacts a parameter to vacuum() and push up its
> calculation for VACUUM and ANALYZE into ExecVacuum().
> This caused a deadlock, so there must be a way that in_vacuum is false
> but vacuum() is called in a nested context.
> Anyway, recalculating it every time in vacuum() fixes it.
>
> Attached is a v12 of the whole vacuum_buffer_usage_limit patch set which
> includes a commit to fix the bug in master and a commit to move relevant
> code from vacuum() up into ExecVacuum().
>
> The logic I suggested earlier for fixing the bug was...not right.
> Attached fix should be right?

David had already pushed a fix, so the patchset had merge conflicts.
Attached v13 should work.

- Melanie
From 5831e5de0b9c9af278ec58d2c9b96410151d0c42 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 5 Apr 2023 20:05:25 -0400
Subject: [PATCH v13 1/3] Push vacuum() setup code up into ExecVacuum()

Much of the sanity checking and setup done in vacuum() code was specific
to explicit VACUUM and ANALYZE. Move that code up into ExecVacuum().
While we are at it, allocate VACUUM/ANALYZE's BufferAccessStrategy in
ExecVacuum() and pass it into vacuum() instead of expecting vacuum() to
make it. This mimics autovacuum's behavior. To do this, create the
relevant memory context in ExecVacuum() and pass it in as a new
parameter to vacuum().
---
 src/backend/commands/vacuum.c       | 191 +++++++++++++++-------------
 src/backend/postmaster/autovacuum.c |  17 ++-
 src/include/commands/vacuum.h       |   3 +-
 3 files changed, 115 insertions(+), 96 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2c31745fbc..cde5ee41a5 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -105,6 +105,7 @@ void
 ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 {
 	VacuumParams params;
+	BufferAccessStrategy bstrategy = NULL;
 	bool		verbose = false;
 	bool		skip_locked = false;
 	bool		analyze = false;
@@ -115,8 +116,12 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		process_toast = true;
 	bool		skip_database_stats = false;
 	bool		only_database_stats = false;
+	MemoryContext vac_context;
 	ListCell   *lc;
 
+	const char *stmttype;
+	volatile bool in_outer_xact;
+
 	/* index_cleanup and truncate values unspecified for now */
 	params.index_cleanup = VACOPTVALUE_UNSPECIFIED;
 	params.truncate = VACOPTVALUE_UNSPECIFIED;
@@ -254,6 +259,42 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		}
 	}
 
+
+	/*
+	 * Sanity check DISABLE_PAGE_SKIPPING option.
+	 */
+	if ((params.options & VACOPT_FULL) != 0 &&
+		(params.options & VACOPT_DISABLE_PAGE_SKIPPING) != 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL")));
+
+	/* sanity check for PROCESS_TOAST */
+	if ((params.options & VACOPT_FULL) != 0 &&
+		(params.options & VACOPT_PROCESS_TOAST) == 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("PROCESS_TOAST required with VACUUM FULL")));
+
+	/* sanity check for ONLY_DATABASE_STATS */
+	if (params.options & VACOPT_ONLY_DATABASE_STATS)
+	{
+		Assert(params.options & VACOPT_VACUUM);
+		if (vacstmt->rels != NIL)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("ONLY_DATABASE_STATS cannot be specified with a list of tables")));
+		/* don't require people to turn off PROCESS_TOAST/MAIN explicitly */
+		if (params.options & ~(VACOPT_VACUUM |
+								VACOPT_VERBOSE |
+								VACOPT_PROCESS_MAIN |
+								VACOPT_PROCESS_TOAST |
+								VACOPT_ONLY_DATABASE_STATS))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("ONLY_DATABASE_STATS cannot be specified with other VACUUM options")));
+	}
+
 	/*
 	 * All freeze ages are zero if the FREEZE option is given; otherwise pass
 	 * them as -1 which means to use the default values.
@@ -279,8 +320,62 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	/* user-invoked vacuum uses VACOPT_VERBOSE instead of log_min_duration */
 	params.log_min_duration = -1;
 
+	stmttype = (params.options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
+
+	/*
+	 * We cannot run VACUUM inside a user transaction block; if we were inside
+	 * a transaction, then our commit- and start-transaction-command calls
+	 * would not have the intended effect!	There are numerous other subtle
+	 * dependencies on this, too.
+	 *
+	 * ANALYZE (without VACUUM) can run either way.
+	 */
+	if (params.options & VACOPT_VACUUM)
+	{
+		PreventInTransactionBlock(isTopLevel, stmttype);
+		in_outer_xact = false;
+	}
+	else
+		in_outer_xact = IsInTransactionBlock(isTopLevel);
+
+	/*
+	 * Create special memory context for cross-transaction storage.
+	 *
+	 * Since it is a child of PortalContext, it will go away eventually even
+	 * if we suffer an error; there's no need for special abort cleanup logic.
+	 */
+	vac_context = AllocSetContextCreate(PortalContext,
+										"Vacuum",
+										ALLOCSET_DEFAULT_SIZES);
+
+	/*
+	 * Make a buffer strategy object in the cross-transaction memory context.
+	 * We needn't bother making this for VACUUM (FULL) or VACUUM
+	 * (ONLY_DATABASE_STATS) as they'll not make use of it.  VACUUM (FULL,
+	 * ANALYZE) is possible, so we'd better ensure that we make a strategy when
+	 * we see ANALYZE.
+	 */
+	if ((params.options & (VACOPT_ONLY_DATABASE_STATS |
+							 VACOPT_FULL)) == 0 ||
+		 (params.options & VACOPT_ANALYZE) != 0)
+	{
+
+		MemoryContext old_context = MemoryContextSwitchTo(vac_context);
+
+		bstrategy = GetAccessStrategy(BAS_VACUUM);
+		MemoryContextSwitchTo(old_context);
+	}
+
 	/* Now go through the common routine */
-	vacuum(vacstmt->rels, &params, NULL, isTopLevel);
+	vacuum(vacstmt->rels, &params, bstrategy, vac_context, isTopLevel,
+		   in_outer_xact);
+
+	/*
+	 * Clean up working storage --- note we must do this after
+	 * StartTransactionCommand, else we might be trying to delete the active
+	 * context!
+	 */
+	MemoryContextDelete(vac_context);
 }
 
 /*
@@ -304,35 +399,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
  */
 void
 vacuum(List *relations, VacuumParams *params,
-	   BufferAccessStrategy bstrategy, bool isTopLevel)
+	   BufferAccessStrategy bstrategy, MemoryContext vac_context,
+	   bool isTopLevel, bool in_outer_xact)
 {
-	static bool in_vacuum = false;
 
-	MemoryContext vac_context;
 	const char *stmttype;
-	volatile bool in_outer_xact,
-				use_own_xacts;
+	static bool in_vacuum = false;
+	volatile bool use_own_xacts;
 
 	Assert(params != NULL);
 
 	stmttype = (params->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
 
-	/*
-	 * We cannot run VACUUM inside a user transaction block; if we were inside
-	 * a transaction, then our commit- and start-transaction-command calls
-	 * would not have the intended effect!	There are numerous other subtle
-	 * dependencies on this, too.
-	 *
-	 * ANALYZE (without VACUUM) can run either way.
-	 */
-	if (params->options & VACOPT_VACUUM)
-	{
-		PreventInTransactionBlock(isTopLevel, stmttype);
-		in_outer_xact = false;
-	}
-	else
-		in_outer_xact = IsInTransactionBlock(isTopLevel);
-
 	/*
 	 * Check for and disallow recursive calls.  This could happen when VACUUM
 	 * FULL or ANALYZE calls a hostile index expression that itself calls
@@ -344,69 +422,6 @@ vacuum(List *relations, VacuumParams *params,
 				 errmsg("%s cannot be executed from VACUUM or ANALYZE",
 						stmttype)));
 
-	/*
-	 * Sanity check DISABLE_PAGE_SKIPPING option.
-	 */
-	if ((params->options & VACOPT_FULL) != 0 &&
-		(params->options & VACOPT_DISABLE_PAGE_SKIPPING) != 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL")));
-
-	/* sanity check for PROCESS_TOAST */
-	if ((params->options & VACOPT_FULL) != 0 &&
-		(params->options & VACOPT_PROCESS_TOAST) == 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("PROCESS_TOAST required with VACUUM FULL")));
-
-	/* sanity check for ONLY_DATABASE_STATS */
-	if (params->options & VACOPT_ONLY_DATABASE_STATS)
-	{
-		Assert(params->options & VACOPT_VACUUM);
-		if (relations != NIL)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("ONLY_DATABASE_STATS cannot be specified with a list of tables")));
-		/* don't require people to turn off PROCESS_TOAST/MAIN explicitly */
-		if (params->options & ~(VACOPT_VACUUM |
-								VACOPT_VERBOSE |
-								VACOPT_PROCESS_MAIN |
-								VACOPT_PROCESS_TOAST |
-								VACOPT_ONLY_DATABASE_STATS))
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("ONLY_DATABASE_STATS cannot be specified with other VACUUM options")));
-	}
-
-	/*
-	 * Create special memory context for cross-transaction storage.
-	 *
-	 * Since it is a child of PortalContext, it will go away eventually even
-	 * if we suffer an error; there's no need for special abort cleanup logic.
-	 */
-	vac_context = AllocSetContextCreate(PortalContext,
-										"Vacuum",
-										ALLOCSET_DEFAULT_SIZES);
-
-	/*
-	 * If caller didn't give us a buffer strategy object, make one in the
-	 * cross-transaction memory context.  We needn't bother making this for
-	 * VACUUM (FULL) or VACUUM (ONLY_DATABASE_STATS) as they'll not make use
-	 * of it.  VACUUM (FULL, ANALYZE) is possible, so we'd better ensure that
-	 * we make a strategy when we see ANALYZE.
-	 */
-	if (bstrategy == NULL &&
-		((params->options & (VACOPT_ONLY_DATABASE_STATS |
-							 VACOPT_FULL)) == 0 ||
-		 (params->options & VACOPT_ANALYZE) != 0))
-	{
-		MemoryContext old_context = MemoryContextSwitchTo(vac_context);
-
-		bstrategy = GetAccessStrategy(BAS_VACUUM);
-		MemoryContextSwitchTo(old_context);
-	}
-
 	/*
 	 * Build list of relation(s) to process, putting any new data in
 	 * vac_context for safekeeping.
@@ -578,12 +593,6 @@ vacuum(List *relations, VacuumParams *params,
 		vac_update_datfrozenxid();
 	}
 
-	/*
-	 * Clean up working storage --- note we must do this after
-	 * StartTransactionCommand, else we might be trying to delete the active
-	 * context!
-	 */
-	MemoryContextDelete(vac_context);
 }
 
 /*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index e9ba0dc17c..d2b6f41ead 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -338,7 +338,8 @@ static void relation_needs_vacanalyze(Oid relid, AutoVacOpts *relopts,
 									  bool *dovacuum, bool *doanalyze, bool *wraparound);
 
 static void autovacuum_do_vac_analyze(autovac_table *tab,
-									  BufferAccessStrategy bstrategy);
+									  BufferAccessStrategy bstrategy,
+									  MemoryContext portal_context);
 static AutoVacOpts *extract_autovac_opts(HeapTuple tup,
 										 TupleDesc pg_class_desc);
 static void perform_work_item(AutoVacuumWorkItem *workitem);
@@ -2469,7 +2470,7 @@ do_autovacuum(void)
 			MemoryContextSwitchTo(PortalContext);
 
 			/* have at it */
-			autovacuum_do_vac_analyze(tab, bstrategy);
+			autovacuum_do_vac_analyze(tab, bstrategy, PortalContext);
 
 			/*
 			 * Clear a possible query-cancel signal, to avoid a late reaction
@@ -3142,11 +3143,13 @@ relation_needs_vacanalyze(Oid relid,
  *		Vacuum and/or analyze the specified table
  */
 static void
-autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
+autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy,
+						  MemoryContext portal_context)
 {
 	RangeVar   *rangevar;
 	VacuumRelation *rel;
 	List	   *rel_list;
+	MemoryContext vac_context;
 
 	/* Let pgstat know what we're doing */
 	autovac_report_activity(tab);
@@ -3156,7 +3159,13 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
 	rel = makeVacuumRelation(rangevar, tab->at_relid, NIL);
 	rel_list = list_make1(rel);
 
-	vacuum(rel_list, &tab->at_params, bstrategy, true);
+	vac_context = AllocSetContextCreate(portal_context,
+										"Vacuum",
+										ALLOCSET_DEFAULT_SIZES);
+
+	vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true, false);
+
+	MemoryContextDelete(vac_context);
 }
 
 /*
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index bdfd96cfec..410eb9dece 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -310,7 +310,8 @@ extern PGDLLIMPORT int VacuumCostBalanceLocal;
 /* in commands/vacuum.c */
 extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
 extern void vacuum(List *relations, VacuumParams *params,
-				   BufferAccessStrategy bstrategy, bool isTopLevel);
+				   BufferAccessStrategy bstrategy, MemoryContext vac_context,
+				   bool isTopLevel, bool in_outer_xact);
 extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
 							 int *nindexes, Relation **Irel);
 extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode);
-- 
2.37.2

From 53d068f0ec3f91666bfa62c9a0e7f2e8ca1b21af Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 19 Mar 2023 18:00:28 -0400
Subject: [PATCH v13 3/3] Add buffer-usage-limit option to vacuumdb

---
 doc/src/sgml/ref/vacuumdb.sgml | 13 +++++++++++++
 src/bin/scripts/vacuumdb.c     | 23 +++++++++++++++++++++++
 src/include/commands/vacuum.h  |  3 +++
 3 files changed, 39 insertions(+)

diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 74bac2d4ba..8280cf0fb0 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -278,6 +278,19 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--buffer-usage-limit <replaceable class="parameter">buffer_usage_limit</replaceable></option></term>
+      <listitem>
+       <para>
+        Specifies the
+        <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
+        ring buffer size for a given invocation of <application>vacuumdb</application>.
+        This size is used to calculate the number of shared buffers which will
+        be reused as part of this strategy.  See <xref linkend="sql-vacuum"/>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-n <replaceable class="parameter">schema</replaceable></option></term>
       <term><option>--schema=<replaceable class="parameter">schema</replaceable></option></term>
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 39be265b5b..941eac7727 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -46,6 +46,7 @@ typedef struct vacuumingOptions
 	bool		process_main;
 	bool		process_toast;
 	bool		skip_database_stats;
+	char	   *buffer_usage_limit;
 } vacuumingOptions;
 
 /* object filter options */
@@ -123,6 +124,7 @@ main(int argc, char *argv[])
 		{"no-truncate", no_argument, NULL, 10},
 		{"no-process-toast", no_argument, NULL, 11},
 		{"no-process-main", no_argument, NULL, 12},
+		{"buffer-usage-limit", required_argument, NULL, 13},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -147,6 +149,7 @@ main(int argc, char *argv[])
 	/* initialize options */
 	memset(&vacopts, 0, sizeof(vacopts));
 	vacopts.parallel_workers = -1;
+	vacopts.buffer_usage_limit = NULL;
 	vacopts.no_index_cleanup = false;
 	vacopts.force_index_cleanup = false;
 	vacopts.do_truncate = true;
@@ -266,6 +269,9 @@ main(int argc, char *argv[])
 			case 12:
 				vacopts.process_main = false;
 				break;
+			case 13:
+				vacopts.buffer_usage_limit = pg_strdup(optarg);
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -343,6 +349,11 @@ main(int argc, char *argv[])
 		pg_fatal("cannot use the \"%s\" option with the \"%s\" option",
 				 "no-index-cleanup", "force-index-cleanup");
 
+	/* buffer-usage-limit doesn't make sense with VACUUM FULL */
+	if (vacopts.buffer_usage_limit && vacopts.full)
+		pg_fatal("cannot use the \"%s\" option with the \"%s\" option",
+				 "buffer-usage-limit", "full");
+
 	/* fill cparams except for dbname, which is set below */
 	cparams.pghost = host;
 	cparams.pgport = port;
@@ -550,6 +561,10 @@ vacuum_one_database(ConnParams *cparams,
 		pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
 				 "--parallel", "13");
 
+	if (vacopts->buffer_usage_limit && PQserverVersion(conn) < 160000)
+		pg_fatal("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
+				 "--buffer-usage-limit", "16");
+
 	/* skip_database_stats is used automatically if server supports it */
 	vacopts->skip_database_stats = (PQserverVersion(conn) >= 160000);
 
@@ -1048,6 +1063,13 @@ prepare_vacuum_command(PQExpBuffer sql, int serverVersion,
 								  vacopts->parallel_workers);
 				sep = comma;
 			}
+			if (vacopts->buffer_usage_limit)
+			{
+				Assert(serverVersion >= 160000);
+				appendPQExpBuffer(sql, "%sBUFFER_USAGE_LIMIT '%s'", sep,
+								  vacopts->buffer_usage_limit);
+				sep = comma;
+			}
 			if (sep != paren)
 				appendPQExpBufferChar(sql, ')');
 		}
@@ -1111,6 +1133,7 @@ help(const char *progname)
 	printf(_("      --force-index-cleanup       always remove index entries that point to dead tuples\n"));
 	printf(_("  -j, --jobs=NUM                  use this many concurrent connections to vacuum\n"));
 	printf(_("      --min-mxid-age=MXID_AGE     minimum multixact ID age of tables to vacuum\n"));
+	printf(_("      --buffer-usage-limit=BUFSIZE size of ring buffer used for vacuum\n"));
 	printf(_("      --min-xid-age=XID_AGE       minimum transaction ID age of tables to vacuum\n"));
 	printf(_("      --no-index-cleanup          don't remove index entries that point to dead tuples\n"));
 	printf(_("      --no-process-main           skip the main relation\n"));
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 410eb9dece..34de657e50 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -213,6 +213,9 @@ typedef enum VacOptValue
  *
  * Note that at least one of VACOPT_VACUUM and VACOPT_ANALYZE must be set
  * in options.
+ *
+ * When adding a new VacuumParam member, consider adding it to vacuumdb as
+ * well.
  */
 typedef struct VacuumParams
 {
-- 
2.37.2

From 399192400ab1e8f8bb13b4cbb17141beba503339 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 5 Apr 2023 17:22:02 -0400
Subject: [PATCH v13 2/3]  Add VACUUM BUFFER_USAGE_LIMIT option and GUC

Add GUC, vacuum_buffer_usage_limit, and VACUUM option,
BUFFER_USAGE_LIMIT, through which the user can specify the maximum size
to use for buffers for VACUUM, ANALYZE, and autovacuum. The size is
converted into a number of shared buffers which are tracked in a
BufferAccessStrategyData object. The explicit VACUUM option, when
specified, overrides the GUC value, unless it is specified as -1.

Reviewed-by: David Rowley <dgrowle...@gmail.com>
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Justin Pryzby <pry...@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/flat/20230111182720.ejifsclfwymw2reb%40awork3.anarazel.de
---
 doc/src/sgml/config.sgml                      | 30 ++++++
 doc/src/sgml/ref/analyze.sgml                 | 18 ++++
 doc/src/sgml/ref/vacuum.sgml                  | 22 +++++
 src/backend/commands/vacuum.c                 | 95 ++++++++++++++++++-
 src/backend/commands/vacuumparallel.c         | 14 ++-
 src/backend/postmaster/autovacuum.c           | 13 ++-
 src/backend/storage/buffer/README             |  4 +
 src/backend/storage/buffer/freelist.c         | 48 ++++++++--
 src/backend/utils/init/globals.c              |  5 +-
 src/backend/utils/misc/guc_tables.c           | 11 +++
 src/backend/utils/misc/postgresql.conf.sample |  4 +
 src/bin/psql/tab-complete.c                   |  4 +-
 src/include/miscadmin.h                       | 13 +++
 src/include/storage/bufmgr.h                  |  6 ++
 src/include/utils/guc_hooks.h                 |  3 +
 src/test/regress/expected/vacuum.out          |  4 +
 src/test/regress/sql/vacuum.sql               |  3 +
 17 files changed, 283 insertions(+), 14 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index bcc49aec45..1d75ec62df 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2001,6 +2001,36 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-vacuum-buffer-usage-limit" xreflabel="vacuum_buffer_usage_limit">
+      <term>
+       <varname>vacuum_buffer_usage_limit</varname> (<type>integer</type>)
+       <indexterm>
+        <primary><varname>vacuum_buffer_usage_limit</varname> configuration parameter</primary>
+       </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the size of <varname>shared_buffers</varname> to be reused
+        for each backend participating in a given invocation of
+        <command>VACUUM</command> or <command>ANALYZE</command> or in
+        autovacuum.  This size is converted to the number of shared buffers
+        which will be reused as part of a
+        <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>.
+        A setting of <literal>0</literal> will allow the operation to use any
+        number of <varname>shared_buffers</varname>, whereas
+        <literal>-1</literal> will set the size to a default of <literal>256 KB</literal>.
+        The maximum size is <literal>16 GB</literal> and the minimum size is <literal>128 KB</literal>.
+        If the specified size would exceed 1/8 the size of
+        <varname>shared_buffers</varname>, it is silently capped.
+        The default value is <literal>-1</literal>.  If this value is specified
+        without units, it is taken as kilobytes. This parameter can be set at
+        any time.  It can be overridden for
+        <xref linkend="sql-vacuum"/> and <xref linkend="sql-analyze"/>
+        when passing the <option>BUFFER_USAGE_LIMIT</option> option.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-logical-decoding-work-mem" xreflabel="logical_decoding_work_mem">
       <term><varname>logical_decoding_work_mem</varname> (<type>integer</type>)
       <indexterm>
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 2f94e89cb0..16cde8668e 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -28,6 +28,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 
     VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
     SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
+    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -95,6 +96,23 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>BUFFER_USAGE_LIMIT</literal></term>
+    <listitem>
+     <para>
+      Specifies the
+      <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
+      ring buffer size for <command>ANALYZE</command>.  This size is used to
+      calculate the number of shared buffers which will be reused as part of
+      this strategy.  <literal>0</literal> disables use of a
+      <literal>Buffer Access Strategy</literal>.  <literal>-1</literal>
+      indicates that <command>ANALYZE</command> should fall back to the value
+      specified by <xref linkend="guc-vacuum-buffer-usage-limit"/>.
+     </para>
+    </listitem>
+   </varlistentry>
+
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index b6d30b5764..c1ddf88cf8 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -39,6 +39,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     PARALLEL <replaceable class="parameter">integer</replaceable>
     SKIP_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
     ONLY_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
+    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -345,6 +346,27 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>BUFFER_USAGE_LIMIT</literal></term>
+    <listitem>
+     <para>
+      Specifies the
+      <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
+      ring buffer size for <command>VACUUM</command>.  This size is used to
+      calculate the number of shared buffers which will be reused as part of
+      this strategy.  <literal>0</literal> disables use of a
+      <literal>Buffer Access Strategy</literal>.  <literal>-1</literal>
+      indicates that <command>VACUUM</command> should fall back to the value
+      specified by <xref linkend="guc-vacuum-buffer-usage-limit"/>.
+      This option can't be used with the <option>FULL</option> option or
+      <option>ONLY_DATABASE_STATS</option> option.  If
+      <option>ANALYZE</option> is also specified, the
+      <option>BUFFER_USAGE_LIMIT</option> value is used for both the vacuum
+      and analyze stages.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index cde5ee41a5..5427e2dedc 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -56,6 +56,7 @@
 #include "utils/acl.h"
 #include "utils/fmgroids.h"
 #include "utils/guc.h"
+#include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 #include "utils/pg_rusage.h"
 #include "utils/snapmgr.h"
@@ -95,6 +96,29 @@ static VacOptValue get_vacoptval_from_boolean(DefElem *def);
 static bool vac_tid_reaped(ItemPointer itemptr, void *state);
 static int	vac_cmp_itemptr(const void *left, const void *right);
 
+/*
+ * GUC check function to ensure GUC value specified is within the allowable
+ * range. This should match the checking in ExecVacuum().
+ */
+bool
+check_vacuum_buffer_usage_limit(int *newval, void **extra,
+								GucSource source)
+{
+	/* Allow specifying the default or disabling Buffer Access Strategy */
+	if (*newval == -1 || *newval == 0)
+		return true;
+
+	/* Value upper and lower hard limits are inclusive */
+	if (*newval >= MIN_BAS_VAC_RING_SIZE_KB && *newval <= MAX_BAS_VAC_RING_SIZE_KB)
+		return true;
+
+	/* Value does not fall within any allowable range */
+	GUC_check_errdetail("\"vacuum_buffer_usage_limit\" must be 0 or between %d KB and %d KB",
+						MIN_BAS_VAC_RING_SIZE_KB, MAX_BAS_VAC_RING_SIZE_KB);
+
+	return false;
+}
+
 /*
  * Primary entry point for manual VACUUM and ANALYZE commands
  *
@@ -114,6 +138,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		disable_page_skipping = false;
 	bool		process_main = true;
 	bool		process_toast = true;
+	/* by default use buffer access strategy with default size */
+	int			ring_size = -1;
 	bool		skip_database_stats = false;
 	bool		only_database_stats = false;
 	MemoryContext vac_context;
@@ -139,6 +165,46 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			verbose = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "skip_locked") == 0)
 			skip_locked = defGetBoolean(opt);
+		else if (strcmp(opt->defname, "buffer_usage_limit") == 0)
+		{
+			const char *hintmsg;
+			int			result;
+			char	   *vac_buffer_size;
+
+			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, &hintmsg))
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("value: \"%s\": is invalid for buffer_usage_limit",
+								vac_buffer_size),
+						 hintmsg ? errhint("%s", _(hintmsg)) : 0));
+			}
+
+			/*
+			 * Check that the specified size falls within the hard upper and
+			 * lower limits if it is not -1 or 0.
+			 */
+			if (result != -1 && result != 0 &&
+				(result < MIN_BAS_VAC_RING_SIZE_KB || result > MAX_BAS_VAC_RING_SIZE_KB))
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("buffer_usage_limit for a vacuum must be 0 or between %d KB and %d KB",
+								MIN_BAS_VAC_RING_SIZE_KB, MAX_BAS_VAC_RING_SIZE_KB)));
+			}
+
+			ring_size = result;
+		}
 		else if (!vacstmt->is_vacuumcmd)
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -243,6 +309,16 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("VACUUM FULL cannot be performed in parallel")));
 
+	if ((params.options & VACOPT_FULL) && ring_size > -1)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("BUFFER_USAGE_LIMIT cannot be specified for VACUUM FULL")));
+
+	if ((params.options & VACOPT_ONLY_DATABASE_STATS) && ring_size > -1)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("BUFFER_USAGE_LIMIT cannot be specified for VACUUM ONLY_DATABASE_STATS")));
+
 	/*
 	 * Make sure VACOPT_ANALYZE is specified if any column lists are present.
 	 */
@@ -362,7 +438,24 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 
 		MemoryContext old_context = MemoryContextSwitchTo(vac_context);
 
-		bstrategy = GetAccessStrategy(BAS_VACUUM);
+		Assert(ring_size >= -1);
+
+		/*
+		 * If explicit VACUUM or ANALYZE specified a non-default
+		 * BUFFER_USAGE_LIMIT, that overrides the value of GUC
+		 * VacuumBufferUsageLimit. If VacuumBufferUsageLimit is set to
+		 * something other than the default, use its value. Otherwise, use the
+		 * default sizes for a Buffer Access Strategy. Note that autovacuum
+		 * will have set VacuumParams->ring_size to the value it derived from
+		 * VacuumBufferUsageLimit.
+		 */
+		if (ring_size > -1)
+			bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, ring_size);
+		else if (VacuumBufferUsageLimit > -1)
+			bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, VacuumBufferUsageLimit);
+		else
+			bstrategy = GetAccessStrategy(BAS_VACUUM);
+
 		MemoryContextSwitchTo(old_context);
 	}
 
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 563117a8f6..614a35779a 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -87,6 +87,12 @@ typedef struct PVShared
 	 */
 	int			maintenance_work_mem_worker;
 
+	/*
+	 * The number of buffers each worker's Buffer Access Strategy ring should
+	 * contain.
+	 */
+	int			ring_nbuffers;
+
 	/*
 	 * Shared vacuum cost balance.  During parallel vacuum,
 	 * VacuumSharedCostBalance points to this value and it accumulates the
@@ -365,6 +371,9 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
 		maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
 		maintenance_work_mem;
 
+	/* Use the same buffer size for all workers */
+	shared->ring_nbuffers = StrategyGetBufferCount(bstrategy);
+
 	pg_atomic_init_u32(&(shared->cost_balance), 0);
 	pg_atomic_init_u32(&(shared->active_nworkers), 0);
 	pg_atomic_init_u32(&(shared->idx), 0);
@@ -1018,8 +1027,9 @@ 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. */
+	pvs.bstrategy = GetAccessStrategyWithSize(BAS_VACUUM,
+											  shared->ring_nbuffers * (BLCKSZ / 1024));
 
 	/* 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 d2b6f41ead..421adc7ad7 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2290,9 +2290,18 @@ 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.
+	 * 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.
+	 *
+	 * XXX: In the future we may want to react to changes in
+	 * VacuumBufferUsageLimit while vacuuming.
 	 */
-	bstrategy = GetAccessStrategy(BAS_VACUUM);
+	if (VacuumBufferUsageLimit > -1)
+		bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, VacuumBufferUsageLimit);
+	else
+		bstrategy = GetAccessStrategy(BAS_VACUUM);
 
 	/*
 	 * create a memory context to act as fake PortalContext, so that the
diff --git a/src/backend/storage/buffer/README b/src/backend/storage/buffer/README
index a775276ff2..d7c9d99af2 100644
--- a/src/backend/storage/buffer/README
+++ b/src/backend/storage/buffer/README
@@ -236,6 +236,10 @@ buffers were sent to the freelist, which was effectively a buffer ring of 1
 buffer, resulting in excessive WAL flushing.  Allowing VACUUM to update
 256KB between WAL flushes should be more efficient.
 
+In v16, the 256KB ring was made configurable by way of the
+vacuum_buffer_usage_limit GUC and the BUFFER_USAGE_LIMIT option to VACUUM and
+ANALYZE.
+
 Bulk writes work similarly to VACUUM.  Currently this applies only to
 COPY IN and CREATE TABLE AS SELECT.  (Might it be interesting to make
 seqscan UPDATE and DELETE use the bulkwrite strategy?)  For bulk writes
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index f122709fbe..0c1ce8b93f 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -540,8 +540,7 @@ StrategyInitialize(bool init)
 BufferAccessStrategy
 GetAccessStrategy(BufferAccessStrategyType btype)
 {
-	BufferAccessStrategy strategy;
-	int			nbuffers;
+	int			ring_size_kb;
 
 	/*
 	 * Select ring size to use.  See buffer/README for rationales.
@@ -556,13 +555,13 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			nbuffers = 256 * 1024 / BLCKSZ;
+			ring_size_kb = 256;
 			break;
 		case BAS_BULKWRITE:
-			nbuffers = 16 * 1024 * 1024 / BLCKSZ;
+			ring_size_kb = 16 * 1024;
 			break;
 		case BAS_VACUUM:
-			nbuffers = 256 * 1024 / BLCKSZ;
+			ring_size_kb = 256;
 			break;
 
 		default:
@@ -571,8 +570,36 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;		/* keep compiler quiet */
 	}
 
+	return GetAccessStrategyWithSize(btype, ring_size_kb);
+}
+
+/*
+ * GetAccessStrategyWithSize -- create a BufferAccessStrategy object with a
+ * 						number of buffers equivalent to the passed in size
+ */
+BufferAccessStrategy
+GetAccessStrategyWithSize(BufferAccessStrategyType btype, int ring_size_kb)
+{
+	int			blcksz_kb = BLCKSZ / 1024;
+	int			nbuffers;
+	int			sb_limit_kb;
+	BufferAccessStrategy strategy;
+
+	/* Default nbuffers should have resulted in calling GetAccessStrategy() */
+	Assert(ring_size_kb >= 0);
+
+	if (ring_size_kb == 0)
+		return NULL;
+
+	Assert(blcksz_kb > 0);
+
 	/* Make sure ring isn't an undue fraction of shared buffers */
-	nbuffers = Min(NBuffers / 8, nbuffers);
+	sb_limit_kb = NBuffers / 8 * blcksz_kb;
+	ring_size_kb = Min(sb_limit_kb, ring_size_kb);
+
+	nbuffers = ring_size_kb / blcksz_kb;
+
+	Assert(nbuffers > 0);
 
 	/* Allocate the object and initialize all elements to zeroes */
 	strategy = (BufferAccessStrategy)
@@ -586,6 +613,15 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 	return strategy;
 }
 
+/*
+ * StrategyGetBufferCount -- an accessor for the number of buffers in the ring
+ */
+int
+StrategyGetBufferCount(BufferAccessStrategy strategy)
+{
+	return strategy->nbuffers;
+}
+
 /*
  * FreeAccessStrategy -- release a BufferAccessStrategy object
  *
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..059a097bef 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -139,7 +139,10 @@ int			max_worker_processes = 8;
 int			max_parallel_workers = 8;
 int			MaxBackends = 0;
 
-int			VacuumCostPageHit = 1;	/* GUC parameters for vacuum */
+/* GUC parameters for vacuum */
+int			VacuumBufferUsageLimit = -1;
+
+int			VacuumCostPageHit = 1;
 int			VacuumCostPageMiss = 2;
 int			VacuumCostPageDirty = 20;
 int			VacuumCostLimit = 200;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8062589efd..5bad2b540f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2224,6 +2224,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"vacuum_buffer_usage_limit", PGC_USERSET, RESOURCES_MEM,
+			gettext_noop("Sets the buffer pool size for VACUUM, ANALYZE, and autovacuum."),
+			NULL,
+			GUC_UNIT_KB
+		},
+		&VacuumBufferUsageLimit,
+		-1, -1, MAX_BAS_VAC_RING_SIZE_KB,
+		check_vacuum_buffer_usage_limit, 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 ee49ca3937..cdbcb95a07 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -157,6 +157,10 @@
 					#   mmap
 					# (change requires restart)
 #min_dynamic_shared_memory = 0MB	# (change requires restart)
+#vacuum_buffer_usage_limit = -1 # size of vacuum and analyze buffer access strategy ring.
+				# -1 to use default,
+				# 0 to disable vacuum buffer access strategy
+				# > 0 to specify size. range 128-16777216
 
 # - Disk -
 
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e38a49e8bd..6614fd2e62 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2662,7 +2662,7 @@ psql_completion(const char *text, int start, int end)
 		 * one word, so the above test is correct.
 		 */
 		if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
-			COMPLETE_WITH("VERBOSE", "SKIP_LOCKED");
+			COMPLETE_WITH("VERBOSE", "SKIP_LOCKED", "BUFFER_USAGE_LIMIT");
 		else if (TailMatches("VERBOSE|SKIP_LOCKED"))
 			COMPLETE_WITH("ON", "OFF");
 	}
@@ -4620,7 +4620,7 @@ psql_completion(const char *text, int start, int end)
 						  "DISABLE_PAGE_SKIPPING", "SKIP_LOCKED",
 						  "INDEX_CLEANUP", "PROCESS_MAIN", "PROCESS_TOAST",
 						  "TRUNCATE", "PARALLEL", "SKIP_DATABASE_STATS",
-						  "ONLY_DATABASE_STATS");
+						  "ONLY_DATABASE_STATS", "BUFFER_USAGE_LIMIT");
 		else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|PROCESS_MAIN|PROCESS_TOAST|TRUNCATE|SKIP_DATABASE_STATS|ONLY_DATABASE_STATS"))
 			COMPLETE_WITH("ON", "OFF");
 		else if (TailMatches("INDEX_CLEANUP"))
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1..2d550f26e0 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -263,6 +263,19 @@ extern PGDLLIMPORT double hash_mem_multiplier;
 extern PGDLLIMPORT int maintenance_work_mem;
 extern PGDLLIMPORT int max_parallel_maintenance_workers;
 
+/*
+ * Global values used by VACUUM and autovacuum.
+ */
+extern PGDLLIMPORT int VacuumBufferUsageLimit;
+
+/*
+ * Upper and lower hard limits for the Buffer Access Strategy ring size
+ * specified by the vacuum_buffer_usage_limit GUC and BUFFER_USAGE_LIMIT option
+ * to VACUUM and ANALYZE.
+ */
+#define MAX_BAS_VAC_RING_SIZE_KB (16 * 1024 * 1024)
+#define MIN_BAS_VAC_RING_SIZE_KB 128
+
 extern PGDLLIMPORT int VacuumCostPageHit;
 extern PGDLLIMPORT int VacuumCostPageMiss;
 extern PGDLLIMPORT int VacuumCostPageDirty;
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 788aa279ba..30daa11f7b 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -260,7 +260,13 @@ extern Size BufferShmemSize(void);
 extern void AtProcExit_LocalBuffers(void);
 
 /* in freelist.c */
+
 extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype);
+extern BufferAccessStrategy GetAccessStrategyWithSize(
+													  BufferAccessStrategyType btype,
+													  int ring_size_kb);
+extern int	StrategyGetBufferCount(BufferAccessStrategy strategy);
+
 extern void FreeAccessStrategy(BufferAccessStrategy strategy);
 
 
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index aeb3663071..1893292634 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -33,6 +33,9 @@ extern bool check_autovacuum_max_workers(int *newval, void **extra,
 										 GucSource source);
 extern bool check_autovacuum_work_mem(int *newval, void **extra,
 									  GucSource source);
+
+extern bool check_vacuum_buffer_usage_limit(int *newval, void **extra,
+											GucSource source);
 extern bool check_backtrace_functions(char **newval, void **extra,
 									  GucSource source);
 extern void assign_backtrace_functions(const char *newval, void *extra);
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index e5a312182e..fe675c5a2e 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -350,6 +350,10 @@ SELECT t.relfilenode = :toast_filenode AS is_same_toast_filenode
  f
 (1 row)
 
+-- BUFFER_USAGE_LIMIT integer overflow error
+VACUUM (BUFFER_USAGE_LIMIT 10000000000) vac_option_tab;
+ERROR:  value: "10000000000": is invalid for buffer_usage_limit
+HINT:  Value exceeds integer range.
 -- SKIP_DATABASE_STATS option
 VACUUM (SKIP_DATABASE_STATS) vactst;
 -- ONLY_DATABASE_STATS option
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index a1fad43657..fc068b7705 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -272,6 +272,9 @@ SELECT t.relfilenode = :toast_filenode AS is_same_toast_filenode
   FROM pg_class c, pg_class t
   WHERE c.reltoastrelid = t.oid AND c.relname = 'vac_option_tab';
 
+-- BUFFER_USAGE_LIMIT integer overflow error
+VACUUM (BUFFER_USAGE_LIMIT 10000000000) vac_option_tab;
+
 -- SKIP_DATABASE_STATS option
 VACUUM (SKIP_DATABASE_STATS) vactst;
 
-- 
2.37.2

Reply via email to