> On Tue, Feb 28, 2023 at 3:16 AM Bharath Rupireddy
> <bharath.rupireddyforpostg...@gmail.com> wrote:
>
> > On Thu, Jan 12, 2023 at 6:06 AM Andres Freund <and...@anarazel.de> wrote:
> > >
> > > On 2023-01-11 17:26:19 -0700, David G. Johnston wrote:
> > > > Should we just add "ring_buffers" to the existing "shared_buffers" and
> > > > "temp_buffers" settings?
> > >
> > > The different types of ring buffers have different sizes, for good
> > > reasons. So
> > > I don't see that working well. I also think it'd be more often useful to
> > > control this on a statement basis - if you have a parallel import tool
> > > that
> > > starts NCPU COPYs you'd want a smaller buffer than a single threaded
> > > COPY. Of
> > > course each session can change the ring buffer settings, but still.
> >
> > How about having GUCs for each ring buffer (bulk_read_ring_buffers,
> > bulk_write_ring_buffers, vacuum_ring_buffers - ah, 3 more new GUCs)?
> > These options can help especially when statement level controls aren't
> > easy to add (COPY, CREATE TABLE AS/CTAS, REFRESH MAT VIEW/RMV)? If
> > needed users can also set them at the system level. For instance, one
> > can set bulk_write_ring_buffers to other than 16MB or -1 to disable
> > the ring buffer to use shared_buffers and run a bunch of bulk write
> > queries.
In attached v3, I've changed the name of the guc from buffer_usage_limit
to vacuum_buffer_usage_limit, since it is only used for vacuum and
autovacuum.
I haven't added the other suggested strategy gucs, as those could easily
be done in a future patchset.
I've also changed GetAccessStrategyExt() to GetAccessStrategyWithSize()
-- similar to initArrayResultWithSize().
And I've added tab completion for BUFFER_USAGE_LIMIT so that it is
easier to try out my patch.
Most of the TODOs in the code are related to the question of how
autovacuum uses the guc vacuum_buffer_usage_limit. autovacuum creates
the buffer access strategy ring in do_autovacuum() before looping
through and vacuuming tables. It passes this strategy object on to
vacuum(). Since we reuse the same strategy object for all tables in a
given invocation of do_autovacuum(), only failsafe autovacuum would
change buffer access strategies. This is probably okay, but it does mean
that the table-level VacuumParams variable, ring_size, means something
different for autovacuum than vacuum. Autovacuum workers will always
have set it to -1. We won't ever reach code in vacuum() which relies on
VacuumParams->ring_size as long as autovacuum workers pass a non-NULL
BufferAccessStrategy object to vacuum(), though.
- Melanie
From 6f40d87f4f462d48a67721260be7e30a7438520e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 22 Feb 2023 12:26:01 -0500
Subject: [PATCH v3 2/3] use shared buffers when failsafe active
---
src/backend/access/heap/vacuumlazy.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8f14cf85f3..b319a244d5 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2622,6 +2622,11 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
if (unlikely(vacuum_xid_failsafe_check(&vacrel->cutoffs)))
{
vacrel->failsafe_active = true;
+ /*
+ * 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 139e71241729d7304188dffb603e6f43db0bf67c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 22 Feb 2023 15:28:34 -0500
Subject: [PATCH v3 3/3] add vacuum option to specify ring_size and guc
---
src/backend/commands/vacuum.c | 51 ++++++++++++++++++-
src/backend/commands/vacuumparallel.c | 6 ++-
src/backend/postmaster/autovacuum.c | 15 +++++-
src/backend/storage/buffer/README | 2 +
src/backend/storage/buffer/freelist.c | 40 +++++++++++++++
src/backend/utils/init/globals.c | 2 +
src/backend/utils/misc/guc_tables.c | 11 ++++
src/backend/utils/misc/postgresql.conf.sample | 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 ++
12 files changed, 136 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index c62be52e3b..22fe42ee2e 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -127,6 +127,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* By default parallel vacuum is enabled */
params.nworkers = 0;
+ /* by default use buffer access strategy with default size */
+ params.ring_size = -1;
+
/* Parse options list */
foreach(lc, vacstmt->options)
{
@@ -210,6 +213,43 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
skip_database_stats = defGetBoolean(opt);
else if (strcmp(opt->defname, "only_database_stats") == 0)
only_database_stats = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "buffer_usage_limit") == 0)
+ {
+ char *vac_buffer_size;
+ int result;
+
+ if (opt->arg == NULL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("buffer_usage_limit option requires a valid value"),
+ parser_errposition(pstate, opt->location)));
+ }
+
+ vac_buffer_size = defGetString(opt);
+
+ if (!parse_int(vac_buffer_size, &result, GUC_UNIT_KB, NULL))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("buffer_usage_limit for a vacuum must be between -1 and %d. %s is invalid.",
+ MAX_BAS_RING_SIZE_KB, vac_buffer_size),
+ parser_errposition(pstate, opt->location)));
+ }
+
+ /* check for out-of-bounds */
+ if (result < -1 || result > MAX_BAS_RING_SIZE_KB)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("buffer_usage_limit for a vacuum must be between -1 and %d",
+ MAX_BAS_RING_SIZE_KB),
+ parser_errposition(pstate, opt->location)));
+ }
+
+ params.ring_size = result;
+
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -399,7 +439,16 @@ vacuum(List *relations, VacuumParams *params,
{
MemoryContext old_context = MemoryContextSwitchTo(vac_context);
- bstrategy = GetAccessStrategy(BAS_VACUUM);
+ if (params->ring_size == -1)
+ {
+ if (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..16742f6612 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1012,7 +1012,11 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
pvs.indname = NULL;
pvs.status = PARALLEL_INDVAC_STATUS_INITIAL;
- /* Each parallel VACUUM worker gets its own access strategy */
+ /*
+ * Each parallel VACUUM worker gets its own access strategy
+ * For now, use the default buffer access strategy ring size.
+ * TODO: should this work differently even though they are only for indexes
+ */
pvs.bstrategy = GetAccessStrategy(BAS_VACUUM);
/* Setup error traceback support for ereport() */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c0e2e00a7e..7dbd3b8935 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,13 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;
tab->at_params.is_wraparound = wraparound;
tab->at_params.log_min_duration = log_min_duration;
+
+ // TODO: should this be 0 so that we are sure that vacuum() never
+ // allocates a new bstrategy for us, even if we pass in NULL for that
+ // parameter? maybe could change how failsafe NULLs out bstrategy if
+ // so?
+ tab->at_params.ring_size = 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..bf166bbdf1 100644
--- a/src/backend/storage/buffer/README
+++ b/src/backend/storage/buffer/README
@@ -245,6 +245,8 @@ for WAL flushes. While it's okay for a background vacuum to be slowed by
doing its own WAL flushing, we'd prefer that COPY not be subject to that,
so we let it use up a bit more of the buffer arena.
+TODO: update with info about new option to control the size
+
Background Writer's Processing
------------------------------
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index c690d5f15f..2fc5412e40 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -22,6 +22,7 @@
#include "storage/proc.h"
#define INT_ACCESS_ONCE(var) ((int)(*((volatile int *)&(var))))
+#define bufsize_limit_to_nbuffers(bufsize) (bufsize * 1024 / BLCKSZ)
/*
@@ -586,6 +587,45 @@ GetAccessStrategy(BufferAccessStrategyType btype)
return strategy;
}
+
+BufferAccessStrategy
+GetAccessStrategyWithSize(BufferAccessStrategyType btype, int ring_size)
+{
+ BufferAccessStrategy strategy;
+ int nbuffers;
+
+ /* Default nbuffers should have resulted in calling GetAccessStrategy() */
+ // TODO: this being called ring_size and also nbuffers being called
+ // ring_size in GetAccessStrategy is confusing. Rename the member of
+ // BufferAccessStrategy
+ Assert(ring_size != -1);
+
+ if (ring_size == 0)
+ return NULL;
+
+ Assert(ring_size < MAX_BAS_RING_SIZE_KB);
+
+ // TODO: warn about clamping?
+ if (ring_size < MIN_BAS_RING_SIZE_KB)
+ nbuffers = bufsize_limit_to_nbuffers(MIN_BAS_RING_SIZE_KB);
+ else
+ nbuffers = bufsize_limit_to_nbuffers(ring_size);
+
+ // TODO: make this smaller?
+ nbuffers = Min(NBuffers / 8, nbuffers);
+
+ /* Allocate the object and initialize all elements to zeroes */
+ strategy = (BufferAccessStrategy)
+ palloc0(offsetof(BufferAccessStrategyData, buffers) +
+ nbuffers * sizeof(Buffer));
+
+ /* Set field that didn't start out zero */
+ strategy->btype = btype;
+ strategy->ring_size = nbuffers;
+
+ return strategy;
+}
+
/*
* FreeAccessStrategy -- release a BufferAccessStrategy object
*
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..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..44bd07d109 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 operations employing a buffer access strategy."),
+ 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..36273aa47d 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 and use shared buffers
+ # > 0 to specify size
# - Disk -
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8f12af799b..fab5b79c34 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..06bf66f4f4 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -236,6 +236,7 @@ typedef struct VacuumParams
* disabled.
*/
int nworkers;
+ int ring_size;
} VacuumParams;
/*
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1..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 02e1c5a0d967a6e42138470c05e2071332934165 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 22 Feb 2023 12:06:41 -0500
Subject: [PATCH v3 1/3] 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 2e12baf8eb..c62be52e3b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -74,7 +74,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;
/*
@@ -93,7 +92,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);
@@ -337,7 +336,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.
*/
@@ -403,7 +402,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
@@ -508,7 +506,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;
}
@@ -526,7 +524,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)
{
@@ -1837,7 +1835,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;
@@ -2083,7 +2081,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 */
@@ -2117,7 +2115,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
