Thanks for the review!
Attached is an updated v6.
v6 has some updates and corrections. It has two remaining TODOs in the
code: one is around what value to initialize the ring_size to in
VacuumParams, the other is around whether or not parallel vacuum index
workers should in fact stick with the default buffer access strategy
sizes.
I also separated vacuumdb into its own commit.
I also have addressed Justin's review feedback.
On Sat, Mar 18, 2023 at 2:30 PM Justin Pryzby <pry...@telsasoft.com> wrote:
>
> > Subject: [PATCH v4 3/3] add vacuum[db] option to specify ring_size and guc
>
> > + Specifies the ring buffer size to be used for a given invocation of
> > + <command>VACUUM</command> or instance of autovacuum. This size is
> > + converted to a number of shared buffers which will be reused as
> > part of
>
> I'd say "specifies the size of shared_buffers to be reused as .."
I've included "shared_buffers" in the description.
> > + a <literal>Buffer Access Strategy</literal>. <literal>0</literal>
> > will
> > + disable use of a <literal>Buffer Access Strategy</literal>.
> > + <literal>-1</literal> will set the size to a default of
> > <literal>256
> > + kB</literal>. The maximum ring buffer size is <literal>16
> > GB</literal>.
> > + Though you may set <varname>vacuum_buffer_usage_limit</varname>
> > below
> > + <literal>128 kB</literal>, it will be clamped to <literal>128
> > + kB</literal> at runtime. The default value is
> > <literal>-1</literal>.
> > + This parameter can be set at any time.
>
> GUC docs usually also say something like
> "If this value is specified without units, it is taken as .."
I had updated this in v5 with slightly different wording, but I now am
using the wording you suggested (which does appear standard in the rest
of the docs).
>
> > + is used to calculate a number of shared buffers which will be reused
> > as
>
> *the* number?
updated.
>
> > + <command>VACUUM</command>. The analyze stage and parallel vacuum
> > workers
> > + do not use this size.
>
> I think what you mean is that vacuum's heap scan stage uses the
> strategy, but the index scan/cleanup phases doesn't?
Yes, non-parallel index vacuum and cleanup will use whatever value you
specify but parallel workers make their own buffer access strategy
object. I've updated the docs to indicate that they will use the default
size for this.
>
> > + The size in kB of the ring buffer used for vacuuming. This size is
> > used
> > + to calculate a number of shared buffers which will be reused as
> > part of
>
> *the* number
fixed.
> > +++ b/doc/src/sgml/ref/vacuumdb.sgml
>
> The docs here duplicate the sql-vacuum docs. It seems better to refer
> to the vacuum page for details, like --parallel does.
Good idea.
>
> Unrelated: it would be nice if the client-side options were documented
> separately from the server-side options. Especially due to --jobs and
> --parallel.
Yes, that would be helpful.
> > + 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)));
> > + }
>
> I think these checks could be collapsed into a single ereport().
>
> if !parse_int() || (result < -1 || result > MAX_BAS_RINGSIZE_KB):
> ereport(ERROR,
> errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("buffer_usage_limit for a vacuum must be an integer
> between -1 and %d",
> MAX_BAS_RING_SIZE_KB),
>
> There was a recent, similar, and unrelated suggestion here:
> https://www.postgresql.org/message-id/20230314.135859.260879647537075548.horikyota.ntt%40gmail.com
So, these have been updated/improved in v5. I still didn't combine them.
I see what you are saying about combining them (and I checked the link
you shared), but in this case, having them separate allows me to provide
info using the hintmsg passed to parse_int() about why it failed during
parse_int -- which could be something not related to range. So, I think
it makes sense to keep them separate.
> > +#vacuum_buffer_usage_limit = -1 # size of vacuum buffer access strategy
> > ring.
> > + # -1 to use default,
> > + # 0 to disable vacuum buffer access strategy
> > and use shared buffers
>
> I think it's confusing to say "and use shared buffers", since
> "strategies" also use shared_buffers. It seems better to remove those 4
> words.
Got it. I've gone ahead and done that.
> > @@ -550,6 +563,13 @@ vacuum_one_database(ConnParams *cparams,
> > pg_fatal("cannot use the \"%s\" option on server versions
> > older than PostgreSQL %s",
> > "--parallel", "13");
> >
> > + // TODO: this is a problem: if the user specifies this option with -1
> > in a
> > + // version before 16, it will not produce an error message. it also
> > won't
> > + // do anything, but that still doesn't seem right.
>
> Actually, that seems fine to me. If someone installs v16 vacuumdb, they
> can run it against old servers and specify the option as -1 without it
> failing with an error. I don't know if anyone will find that useful,
> but it doesn't seem unreasonable.
I sort of skirted around this by removing any validation from vacuumdb
(present in v5 and still the case in v6). Now, the parameter is a string
and I check if it is non-NULL when the version is < 16. However, this
will no longer have the property that someone can use v16 vacuumdb and
pass buffer-usage-limit and have it not fail. I think that is okay,
though, since they might be confused thinking it was doing something.
> I still think adding something to the glossary would be good.
>
> Buffer Access Strategy:
> A circular/ring buffer used for reading or writing data pages from/to
> the operating system. Ring buffers are used for sequential scans of
> large tables, VACUUM, COPY FROM, CREATE TABLE AS SELECT, ALTER TABLE,
> and CLUSTER. By using only a limited portion of >shared_buffers<, the
> ring buffer avoids avoids evicting large amounts of data whenever a
> backend performs bulk I/O operations. Use of a ring buffer also forces
> the backend to write out its own dirty pages, rather than leaving them
> behind to be cleaned up by other backends.
Yes, I have taken some ideas from here and added a separate commit
before all the others adding Buffer Access Strategy to the
documentation.
> If there's a larger section added than a glossary entry, the text could
> be promoted from src/backend/storage/buffer/README to doc/.
This is a good idea. I think we provided enough information in the
glossary (as far as users would care) if it weren't for the new
buffer_usage_limit guc, which probably merits more explanation about how
it interacts with buffer access strategies. Since it is only used for
vacuum now, do you think such a thing would belong in VACUUM-related
documentation? Like somewhere in [1]?
On Wed, Mar 15, 2023 at 9:03 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
> On Wed, Mar 15, 2023 at 8:14 PM Justin Pryzby <pry...@telsasoft.com> wrote:
> > On Tue, Mar 14, 2023 at 08:56:58PM -0400, Melanie Plageman wrote:
> > > On Sat, Mar 11, 2023 at 2:16 PM Justin Pryzby <pry...@telsasoft.com>
> > > wrote:
> >
> > > > > @@ -586,6 +587,45 @@ GetAccessStrategy(BufferAccessStrategyType btype)
> > > > > +BufferAccessStrategy
> > > > > +GetAccessStrategyWithSize(BufferAccessStrategyType btype, int
> > > > > ring_size)
> > > >
> > > > Maybe it would make sense for GetAccessStrategy() to call
> > > > GetAccessStrategyWithSize(). Or maybe not.
> > >
> > > You mean instead of having anyone call GetAccessStrategyWithSize()?
> > > We would need to change the signature of GetAccessStrategy() then -- and
> > > there are quite a few callers.
> >
> > I mean to avoid code duplication, GetAccessStrategy() could "Select ring
> > size to use" and then call into GetAccessStrategyWithSize(). Maybe it's
> > counter to your intent or otherwise not worth it to save 8 LOC.
>
> Oh, that's a cool idea. I will think on it.
So, I thought about doing a version of this by adding a helper which did
the allocation of the BufferAccessStrategy object given a number of
buffers that could be called by both GetAccessStrategy() and
GetAccessStrategyWithSize(). I decided not to because I wanted to emit a
debug message if the size of the ring was clamped lower or higher than
the user would expect -- but only do this in GetAccessStrategyWithSize()
since there is no user expectation in the use of GetAccessStrategy().
- Melanie
[1] https://www.postgresql.org/docs/devel/routine-vacuuming.html
From 1be30138fa37bb62980fa68fbbff29ef6bc9bfa4 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 19 Mar 2023 18:00:28 -0400
Subject: [PATCH v6 6/6] Add buffer-usage-limit option to vacuumdb
---
doc/src/sgml/ref/vacuumdb.sgml | 12 ++++++++++++
src/bin/scripts/vacuumdb.c | 21 +++++++++++++++++++++
src/include/commands/vacuum.h | 3 +++
3 files changed, 36 insertions(+)
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 74bac2d4ba..0b4c29f685 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -278,6 +278,18 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--buffer-usage-limit <replaceable class="parameter">buffer_usage_limit</replaceable></option></term>
+ <listitem>
+ <para>
+ The size of the ring buffer used for vacuuming. This size is used to
+ calculate the number of shared buffers which will be reused as part of
+ a <literal>Buffer Access Strategy</literal>. 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..5b2a5d2858 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);
@@ -324,6 +330,9 @@ main(int argc, char *argv[])
if (!vacopts.process_toast)
pg_fatal("cannot use the \"%s\" option when performing only analyze",
"no-process-toast");
+ if (vacopts.buffer_usage_limit)
+ pg_fatal("cannot use the \"%s\" option when performing only analyze",
+ "buffer-usage-limit");
/* allow 'and_analyze' with 'analyze_only' */
}
@@ -550,6 +559,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 +1061,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 +1131,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 5f2a58b2c3..d8fba51d82 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 a2a59567ad38dae1c3013da5a4b359e76f3b75fc Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 19 Mar 2023 13:15:24 -0400
Subject: [PATCH v6 4/6] Rename Buffer Access Strategy->ring_size nbuffers
ring_size sounds like it is in units of bytes when it is actually a
number of buffers. Rename it to nbuffers to make future commit related
to ring size less confusing.
---
src/backend/storage/buffer/freelist.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index c690d5f15f..f122709fbe 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -74,7 +74,7 @@ typedef struct BufferAccessStrategyData
/* Overall strategy type */
BufferAccessStrategyType btype;
/* Number of elements in buffers[] array */
- int ring_size;
+ int nbuffers;
/*
* Index of the "current" slot in the ring, ie, the one most recently
@@ -541,7 +541,7 @@ BufferAccessStrategy
GetAccessStrategy(BufferAccessStrategyType btype)
{
BufferAccessStrategy strategy;
- int ring_size;
+ int nbuffers;
/*
* Select ring size to use. See buffer/README for rationales.
@@ -556,13 +556,13 @@ GetAccessStrategy(BufferAccessStrategyType btype)
return NULL;
case BAS_BULKREAD:
- ring_size = 256 * 1024 / BLCKSZ;
+ nbuffers = 256 * 1024 / BLCKSZ;
break;
case BAS_BULKWRITE:
- ring_size = 16 * 1024 * 1024 / BLCKSZ;
+ nbuffers = 16 * 1024 * 1024 / BLCKSZ;
break;
case BAS_VACUUM:
- ring_size = 256 * 1024 / BLCKSZ;
+ nbuffers = 256 * 1024 / BLCKSZ;
break;
default:
@@ -572,16 +572,16 @@ GetAccessStrategy(BufferAccessStrategyType btype)
}
/* Make sure ring isn't an undue fraction of shared buffers */
- ring_size = Min(NBuffers / 8, ring_size);
+ nbuffers = Min(NBuffers / 8, nbuffers);
/* Allocate the object and initialize all elements to zeroes */
strategy = (BufferAccessStrategy)
palloc0(offsetof(BufferAccessStrategyData, buffers) +
- ring_size * sizeof(Buffer));
+ nbuffers * sizeof(Buffer));
/* Set fields that don't start out zero */
strategy->btype = btype;
- strategy->ring_size = ring_size;
+ strategy->nbuffers = nbuffers;
return strategy;
}
@@ -615,7 +615,7 @@ GetBufferFromRing(BufferAccessStrategy strategy, uint32 *buf_state)
/* Advance to next ring slot */
- if (++strategy->current >= strategy->ring_size)
+ if (++strategy->current >= strategy->nbuffers)
strategy->current = 0;
/*
--
2.37.2
From ea1fbd78532c7ee898dd4d84c197f8422debaf43 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 19 Mar 2023 18:00:08 -0400
Subject: [PATCH v6 5/6] Add vacuum[db] buffer usage limit option and guc
---
doc/src/sgml/config.sgml | 26 ++++++++
doc/src/sgml/ref/vacuum.sgml | 23 ++++++++
src/backend/commands/vacuum.c | 51 +++++++++++++++-
src/backend/commands/vacuumparallel.c | 8 ++-
src/backend/postmaster/autovacuum.c | 17 +++++-
src/backend/storage/buffer/README | 21 +++++--
src/backend/storage/buffer/freelist.c | 59 +++++++++++++++++++
src/backend/utils/init/globals.c | 2 +
src/backend/utils/misc/guc_tables.c | 11 ++++
src/backend/utils/misc/postgresql.conf.sample | 4 ++
src/bin/psql/tab-complete.c | 2 +-
src/include/commands/vacuum.h | 1 +
src/include/miscadmin.h | 1 +
src/include/storage/bufmgr.h | 5 ++
14 files changed, 221 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index fca38a4514..1bf0050d8d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1981,6 +1981,32 @@ 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 a given invocation of <command>VACUUM</command> or instance of
+ autovacuum. This size is converted to the number of shared buffers
+ which will be reused as part of a <literal>Buffer Access
+ Strategy</literal>. <literal>0</literal> will disable use of a
+ <literal>Buffer Access Strategy</literal>. <literal>-1</literal> will
+ set the size to a default of <literal>256 kB</literal>. The maximum
+ ring buffer size is <literal>16 GB</literal>. Though you may set
+ <varname>vacuum_buffer_usage_limit</varname> below <literal>128
+ kB</literal>, it will be clamped to <literal>128 kB</literal> at
+ runtime. 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.
+ </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/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index b6d30b5764..8ab89cfa3c 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,28 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>BUFFER_USAGE_LIMIT</literal></term>
+ <listitem>
+ <para>
+ Specifies the 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 a <literal>Buffer Access Strategy</literal>. <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"/>. The maximum value is
+ <literal>16 GB</literal>. Though you may specify a size smaller than
+ <literal>128</literal>, the value will be clamped to <literal>128
+ kB</literal> at runtime. If this value is specified without units, it is
+ taken as kilobytes. This size applies to a single invocation of
+ <command>VACUUM</command>. Parallel VACUUM workers use the default Buffer
+ Access Strategy ring size during index scan and index cleanup phases of
+ VACUUM.
+ </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 a6aac30529..f434f07dd1 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -128,6 +128,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)
{
@@ -211,6 +214,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;
+ const char *hintmsg;
+
+ 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 for out-of-bounds */
+ if (result < -1 || result > MAX_BAS_RING_SIZE_KB)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("buffer_usage_limit for a vacuum must be between -1 and %d",
+ MAX_BAS_RING_SIZE_KB)));
+ }
+
+ params.ring_size = result;
+
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -400,7 +440,16 @@ vacuum(List *relations, VacuumParams *params,
{
MemoryContext old_context = MemoryContextSwitchTo(vac_context);
- bstrategy = GetAccessStrategy(BAS_VACUUM);
+ if (params->ring_size == -1)
+ {
+ if (vacuum_buffer_usage_limit == -1)
+ bstrategy = GetAccessStrategy(BAS_VACUUM);
+ else
+ bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, vacuum_buffer_usage_limit);
+ }
+ else
+ bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, params->ring_size);
+
MemoryContextSwitchTo(old_context);
}
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index bcd40c80a1..4c19fc095e 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1012,7 +1012,13 @@ 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 each parallel VACUUM worker doing index vacuum get a ring
+ * of the full custom size if we are doing that? Or should we split it
+ * amongst them? Or should we just use the default?
+ */
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..a8657b0b32 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 (vacuum_buffer_usage_limit == -1)
+ bstrategy = GetAccessStrategy(BAS_VACUUM);
+ else
+ bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, vacuum_buffer_usage_limit);
/*
* create a memory context to act as fake PortalContext, so that the
@@ -2881,6 +2887,15 @@ 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 = vacuum_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..d1be1ca5b7 100644
--- a/src/backend/storage/buffer/README
+++ b/src/backend/storage/buffer/README
@@ -229,12 +229,21 @@ update hint bits). In a scan that modifies every page in the scan, like a
bulk UPDATE or DELETE, the buffers in the ring will always be dirtied and
the ring strategy effectively degrades to the normal strategy.
-VACUUM uses a 256KB ring like sequential scans, but dirty pages are not
-removed from the ring. Instead, WAL is flushed if needed to allow reuse of
-the buffers. Before introducing the buffer ring strategy in 8.3, VACUUM's
-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.
+VACUUM's default Buffer Access Strategy uses a 256KB ring like sequential
+scans, but dirty pages are not removed from the ring. Instead, WAL is flushed
+if needed to allow reuse of the buffers. Before introducing the buffer ring
+strategy in 8.3, VACUUM's 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.
+
+As an alternative, VACUUM can use a user-specified ring size. The VACUUM
+parameter "BUFFER_USAGE_LIMIT" and GUC vacuum_buffer_usage_limit can be used to
+specify the amount of shared memory to be used during vacuuming. This size is
+used to calculate the number of buffers in the ring when it is created. A value
+of 0 for vacuum_buffer_usage_limit will disable use of the Buffer Access
+Strategy and allow vacuuming to use shared buffers as normal.
+In failsafe mode, autovacuum will always abandon use of a Buffer Access
+Strategy.
Bulk writes work similarly to VACUUM. Currently this applies only to
COPY IN and CREATE TABLE AS SELECT. (Might it be interesting to make
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index f122709fbe..f0a1b59e2c 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -586,6 +586,65 @@ GetAccessStrategy(BufferAccessStrategyType btype)
return strategy;
}
+static inline int
+bufsize_limit_to_nbuffers(int bufsize_limit_kb)
+{
+ int blcksz_kb = BLCKSZ / 1024;
+
+ Assert(blcksz_kb > 0);
+
+ return bufsize_limit_kb / blcksz_kb;
+}
+
+BufferAccessStrategy
+GetAccessStrategyWithSize(BufferAccessStrategyType btype, int ring_size)
+{
+ BufferAccessStrategy strategy;
+ int nbuffers;
+ int clamped_nbuffers;
+
+ /* Default nbuffers should have resulted in calling GetAccessStrategy() */
+ Assert(ring_size != -1);
+
+ if (ring_size == 0)
+ return NULL;
+
+ Assert(ring_size <= MAX_BAS_RING_SIZE_KB);
+
+ if (ring_size < MIN_BAS_RING_SIZE_KB)
+ {
+ ereport(DEBUG1,
+ (errmsg_internal("Buffer Access Strategy ring_size %d kB has been clamped to minimum %d kB",
+ 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);
+
+ clamped_nbuffers = Min(NBuffers / 8, nbuffers);
+
+ if (clamped_nbuffers < nbuffers)
+ ereport(DEBUG1,
+ (errmsg_internal("active Buffer Access Strategy may use a maximum of %d buffers. %d has been clamped",
+ NBuffers / 8,
+ nbuffers)));
+
+ nbuffers = clamped_nbuffers;
+
+ /* Allocate the object and initialize all elements to zeroes */
+ strategy = (BufferAccessStrategy)
+ palloc0(offsetof(BufferAccessStrategyData, buffers) +
+ nbuffers * sizeof(Buffer));
+
+ /* Set fields that don't start out zero */
+ strategy->btype = btype;
+ strategy->nbuffers = 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..6eca3371bd 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 vacuum_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..883ee29d14 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
},
+ {
+ {"vacuum_buffer_usage_limit", PGC_USERSET, RESOURCES_MEM,
+ gettext_noop("Sets the buffer pool size for VACUUM and autovacuum."),
+ NULL,
+ GUC_UNIT_KB
+ },
+ &vacuum_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..e55a1008f1 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -156,6 +156,10 @@
# mmap
# (change requires restart)
#min_dynamic_shared_memory = 0MB # (change requires restart)
+#vacuum_buffer_usage_limit = -1 # size of vacuum buffer access strategy ring.
+ # -1 to use default,
+ # 0 to disable vacuum buffer access strategy
+ # > 0 to specify size
# - Disk -
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 42e87b9e49..6fd80dd3c3 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -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/commands/vacuum.h b/src/include/commands/vacuum.h
index bdfd96cfec..5f2a58b2c3 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..b572dfcc6c 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 vacuum_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..338de38568 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 GetAccessStrategyWithSize(BufferAccessStrategyType btype, int nbuffers);
extern void FreeAccessStrategy(BufferAccessStrategy strategy);
--
2.37.2
From 52c802a00572b382ee0203b0038dbe509e4ecb72 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 22 Feb 2023 12:26:01 -0500
Subject: [PATCH v6 3/6] use shared buffers when failsafe active
---
doc/src/sgml/config.sgml | 8 +++++---
src/backend/access/heap/vacuumlazy.c | 7 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6..fca38a4514 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9291,9 +9291,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
the failsafe to trigger during any <command>VACUUM</command>.
</para>
<para>
- When the failsafe is triggered, any cost-based delay that is
- in effect will no longer be applied, and further non-essential
- maintenance tasks (such as index vacuuming) are bypassed.
+ When the failsafe is triggered, any cost-based delay that is in effect
+ will no longer be applied, further non-essential maintenance tasks
+ (such as index vacuuming) are bypassed, and any Buffer Access Strategy
+ in use will be abandoned and the vacuum will be free to use as many
+ shared buffers as it needs to finish vacuuming the table.
</para>
<para>
The default is 1.6 billion transactions. Although users can
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8f14cf85f3..3de7976cf6 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2622,6 +2622,13 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
if (unlikely(vacuum_xid_failsafe_check(&vacrel->cutoffs)))
{
vacrel->failsafe_active = true;
+ /*
+ * Abandon use of a buffer access strategy when entering failsafe mode,
+ * as completing the ongoing VACUUM is our top priority.
+ * Assume the caller who allocated the memory for the
+ * BufferAccessStrategy object will free it.
+ */
+ vacrel->bstrategy = NULL;
/* Disable index vacuuming, index cleanup, and heap rel truncation */
vacrel->do_index_vacuuming = false;
--
2.37.2
From 9ee2f4f68fc9314d42244b16d5268324fe7aff53 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 22 Feb 2023 12:06:41 -0500
Subject: [PATCH v6 2/6] remove global variable vac_strategy
---
src/backend/commands/vacuum.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index c54360a6a0..a6aac30529 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -75,7 +75,6 @@ int vacuum_multixact_failsafe_age;
/* A few variables that don't seem worth passing around as parameters */
static MemoryContext vac_context = NULL;
-static BufferAccessStrategy vac_strategy;
/*
@@ -94,7 +93,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
TransactionId lastSaneFrozenXid,
MultiXactId lastSaneMinMulti);
static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
- bool skip_privs);
+ bool skip_privs, BufferAccessStrategy bstrategy);
static double compute_parallel_delay(void);
static VacOptValue get_vacoptval_from_boolean(DefElem *def);
static bool vac_tid_reaped(ItemPointer itemptr, void *state);
@@ -338,7 +337,7 @@ vacuum(List *relations, VacuumParams *params,
in_outer_xact = IsInTransactionBlock(isTopLevel);
/*
- * Due to static variables vac_context, anl_context and vac_strategy,
+ * Due to static variable vac_context
* vacuum() is not reentrant. This matters when VACUUM FULL or ANALYZE
* calls a hostile index expression that itself calls ANALYZE.
*/
@@ -404,7 +403,6 @@ vacuum(List *relations, VacuumParams *params,
bstrategy = GetAccessStrategy(BAS_VACUUM);
MemoryContextSwitchTo(old_context);
}
- vac_strategy = bstrategy;
/*
* Build list of relation(s) to process, putting any new data in
@@ -509,7 +507,7 @@ vacuum(List *relations, VacuumParams *params,
if (params->options & VACOPT_VACUUM)
{
- if (!vacuum_rel(vrel->oid, vrel->relation, params, false))
+ if (!vacuum_rel(vrel->oid, vrel->relation, params, false, bstrategy))
continue;
}
@@ -527,7 +525,7 @@ vacuum(List *relations, VacuumParams *params,
}
analyze_rel(vrel->oid, vrel->relation, params,
- vrel->va_cols, in_outer_xact, vac_strategy);
+ vrel->va_cols, in_outer_xact, bstrategy);
if (use_own_xacts)
{
@@ -1838,7 +1836,7 @@ vac_truncate_clog(TransactionId frozenXID,
* At entry and exit, we are not inside a transaction.
*/
static bool
-vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
+vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs, BufferAccessStrategy bstrategy)
{
LOCKMODE lmode;
Relation rel;
@@ -2084,7 +2082,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
cluster_rel(relid, InvalidOid, &cluster_params);
}
else
- table_relation_vacuum(rel, params, vac_strategy);
+ table_relation_vacuum(rel, params, bstrategy);
}
/* Roll back any GUC changes executed by index functions */
@@ -2118,7 +2116,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
- vacuum_rel(toast_relid, NULL, &toast_vacuum_params, true);
+ vacuum_rel(toast_relid, NULL, &toast_vacuum_params, true, bstrategy);
}
/*
--
2.37.2
From 66a4555afa4bcad41fc6da241f28336e314b8328 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 19 Mar 2023 17:40:11 -0400
Subject: [PATCH v6 1/6] Add Buffer Access Strategy to glossary
Reviewed-by: Justin Pryzby <pry...@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/ZBYDTrD1kyGg%2BHkS%40telsasoft.com
---
doc/src/sgml/glossary.sgml | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/doc/src/sgml/glossary.sgml b/doc/src/sgml/glossary.sgml
index 7c01a541fe..d2168f26b1 100644
--- a/doc/src/sgml/glossary.sgml
+++ b/doc/src/sgml/glossary.sgml
@@ -252,6 +252,28 @@
</glossdef>
</glossentry>
+ <glossentry id="glossary-buffer-access-strategy">
+ <glossterm>Buffer Access Strategy</glossterm>
+ <glossdef>
+ <para>
+ Some operations will access a large number of pages at a time. In order to
+ avoid using all of <varname>shared_buffers</varname> and evicting blocks
+ from buffers in use by other workloads, some operations employ a
+ <varname>Buffer Access Strategy</varname>. A <varname>Buffer Access
+ Strategy</varname> sets up references to a limited number of
+ <varname>shared_buffers</varname> and reuses them circularly. If the
+ operation dirties a buffer that it then must reuse, it must write out that
+ buffer and any associated WAL itself.
+ </para>
+ <para>
+ <varname>Buffer Access Strategies</varname> are used for sequential scans
+ of large tables, VACUUM, COPY operations, CREATE TABLE AS SELECT, ALTER
+ TABLE, CREATE DATABASE, CREATE INDEX, and CLUSTER, amongst other
+ operations.
+ </para>
+ </glossdef>
+ </glossentry>
+
<glossentry id="glossary-cast">
<glossterm>Cast</glossterm>
<glossdef>
--
2.37.2
