On Thu, Mar 2, 2023 at 6:37 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
>
> On Thu, Mar 2, 2023 at 2:36 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
> >
> > On Thu, Mar 2, 2023 at 10:41 AM Melanie Plageman
> > <melanieplage...@gmail.com> wrote:
> > > On another topic, I've just realized that when autovacuuming we only
> > > update tab->at_vacuum_cost_delay/limit from
> > > autovacuum_vacuum_cost_delay/limit for each table (in
> > > table_recheck_autovac()) and then use that to update
> > > MyWorkerInfo->wi_cost_delay/limit. MyWorkerInfo->wi_cost_delay/limit is
> > > what is used to update VacuumCostDelay/Limit in AutoVacuumUpdateDelay().
> > > So, even if we reload the config file in vacuum_delay_point(), if we
> > > don't use the new value of autovacuum_vacuum_cost_delay/limit it will
> > > have no effect for autovacuum.
> >
> > Right, but IIUC wi_cost_limit (and VacuumCostDelayLimit) might be
> > updated. After the autovacuum launcher reloads the config file, it
> > calls autovac_balance_cost() that updates that value of active
> > workers. I'm not sure why we don't update workers' wi_cost_delay,
> > though.
>
> Ah yes, I didn't realize this. Thanks. I went back and did more code
> reading/analysis, and I see no reason why we shouldn't update
> worker->wi_cost_delay to the new value of autovacuum_vac_cost_delay in
> autovac_balance_cost(). Then, as you said, the autovac launcher will
> call autovac_balance_cost() when it reloads the configuration file.
> Then, the next time the autovac worker calls AutoVacuumUpdateDelay(), it
> will update VacuumCostDelay.
>
> > > I started writing a little helper that could be used to update these
> > > workerinfo->wi_cost_delay/limit in vacuum_delay_point(),
> >
> > Since we set vacuum delay parameters for autovacuum workers so that we
> > ration out I/O equally, I think we should keep the current mechanism
> > that the autovacuum launcher sets workers' delay parameters and they
> > update accordingly.
>
> Yes, agreed, it should go in the same place as where we update
> wi_cost_limit (autovac_balance_cost()). I think we should potentially
> rename autovac_balance_cost() because its name and all its comments
> point to its only purpose being to balance the total of the workers
> wi_cost_limits to no more than autovacuum_vacuum_cost_limit. And the
> autovacuum_vacuum_cost_delay doesn't need to be balanced in this way.
>
> Though, since this change on its own would make autovacuum pick up new
> values of autovacuum_vacuum_cost_limit (without having the worker reload
> the config file), I wonder if it makes sense to try and have
> vacuum_delay_point() only reload the config file if it is an explicit
> vacuum or an analyze not being run in an outer transaction (to avoid
> overhead of reloading config file)?
>
> The lifecycle of this different vacuum delay-related gucs and how it
> differs between autovacuum workers and explicit vacuum is quite tangled
> already, though.
So, I've attached a new version of the patch which is quite different
from the previous versions.
In this version I've removed wi_cost_delay from WorkerInfoData. There is
no synchronization of cost_delay amongst workers, so there is no reason
to keep it in shared memory.
One consequence of not updating VacuumCostDelay from wi_cost_delay is
that we have to have a way to keep track of whether or not autovacuum
table options are in use.
This patch does this in a cringeworthy way. I added two global
variables, one to track whether or not cost delay table options are in
use and the other to store the value of the table option cost delay. I
didn't want to use a single variable with a special value to indicate
that table option cost delay is in use because
autovacuum_vacuum_cost_delay already has special values that mean
certain things. My code needs a better solution.
It is worth mentioning that I think that in master,
AutoVacuumUpdateDelay() was incorrectly reading wi_cost_limit and
wi_cost_delay from shared memory without holding a lock.
I've added in a shared lock for reading from wi_cost_limit in this
patch. However, AutoVacuumUpdateLimit() is called unconditionally in
vacuum_delay_point(), which is called quite often (per block-ish), so I
was trying to think if there is a way we could avoid having to check
this shared memory variable on every call to vacuum_delay_point().
Rebalances shouldn't happen very often (done by the launcher when a new
worker is launched and by workers between vacuuming tables). Maybe we
can read from it less frequently?
Also not sure how the patch interacts with failsafe autovac and parallel
vacuum.
- Melanie
From 9b5cbbc0c8f892dde3e220f0945b2c1e0d175b84 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 5 Mar 2023 14:39:16 -0500
Subject: [PATCH v2] Reload config file more often while vacuuming
---
src/backend/commands/vacuum.c | 38 ++++++++---
src/backend/postmaster/autovacuum.c | 97 ++++++++++++++++++++++-------
src/include/postmaster/autovacuum.h | 2 +
3 files changed, 104 insertions(+), 33 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index aa79d9de4d..f6cea30168 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -48,6 +48,7 @@
#include "pgstat.h"
#include "postmaster/autovacuum.h"
#include "postmaster/bgworker_internals.h"
+#include "postmaster/interrupt.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
@@ -75,6 +76,7 @@ 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;
+static bool analyze_in_outer_xact = false;
/*
@@ -309,8 +311,7 @@ vacuum(List *relations, VacuumParams *params,
static bool in_vacuum = false;
const char *stmttype;
- volatile bool in_outer_xact,
- use_own_xacts;
+ volatile bool use_own_xacts;
Assert(params != NULL);
@@ -327,10 +328,10 @@ vacuum(List *relations, VacuumParams *params,
if (params->options & VACOPT_VACUUM)
{
PreventInTransactionBlock(isTopLevel, stmttype);
- in_outer_xact = false;
+ analyze_in_outer_xact = false;
}
else
- in_outer_xact = IsInTransactionBlock(isTopLevel);
+ analyze_in_outer_xact = IsInTransactionBlock(isTopLevel);
/*
* Due to static variables vac_context, anl_context and vac_strategy,
@@ -451,7 +452,7 @@ vacuum(List *relations, VacuumParams *params,
Assert(params->options & VACOPT_ANALYZE);
if (IsAutoVacuumWorkerProcess())
use_own_xacts = true;
- else if (in_outer_xact)
+ else if (analyze_in_outer_xact)
use_own_xacts = false;
else if (list_length(relations) > 1)
use_own_xacts = true;
@@ -469,7 +470,7 @@ vacuum(List *relations, VacuumParams *params,
*/
if (use_own_xacts)
{
- Assert(!in_outer_xact);
+ Assert(!analyze_in_outer_xact);
/* ActiveSnapshot is not set by autovacuum */
if (ActiveSnapshotSet())
@@ -521,7 +522,7 @@ vacuum(List *relations, VacuumParams *params,
}
analyze_rel(vrel->oid, vrel->relation, params,
- vrel->va_cols, in_outer_xact, vac_strategy);
+ vrel->va_cols, analyze_in_outer_xact, vac_strategy);
if (use_own_xacts)
{
@@ -544,6 +545,7 @@ vacuum(List *relations, VacuumParams *params,
{
in_vacuum = false;
VacuumCostActive = false;
+ analyze_in_outer_xact = false;
}
PG_END_TRY();
@@ -2214,10 +2216,28 @@ vacuum_delay_point(void)
WAIT_EVENT_VACUUM_DELAY);
ResetLatch(MyLatch);
+ /*
+ * Reload the configuration file if requested. This allows changes to
+ * [autovacuum_]vacuum_cost_limit and [autovacuum_]vacuum_cost_delay
+ * to take effect while a table is being vacuumed or analyzed.
+ */
+ if (ConfigReloadPending && !analyze_in_outer_xact)
+ {
+ ConfigReloadPending = false;
+ ProcessConfigFile(PGC_SIGHUP);
+ AutoVacuumUpdateDelay();
+ }
+
VacuumCostBalance = 0;
- /* update balance values for workers */
- AutoVacuumUpdateDelay();
+ /*
+ * Update balance values for workers. We must always do this in case
+ * the autovacuum launcher has done a rebalance (as it does when
+ * launching a new worker).
+ */
+ AutoVacuumUpdateLimit();
+
+ VacuumCostActive = (VacuumCostDelay > 0);
/* Might have gotten an interrupt while sleeping */
CHECK_FOR_INTERRUPTS();
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ff6149a179..78b4233241 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -139,6 +139,9 @@ int Log_autovacuum_min_duration = 600000;
static bool am_autovacuum_launcher = false;
static bool am_autovacuum_worker = false;
+static bool av_use_table_option_cost_delay = false;
+static double av_table_option_cost_delay = 0;
+
/* Flags set by signal handlers */
static volatile sig_atomic_t got_SIGUSR2 = false;
@@ -189,7 +192,6 @@ typedef struct autovac_table
{
Oid at_relid;
VacuumParams at_params;
- double at_vacuum_cost_delay;
int at_vacuum_cost_limit;
bool at_dobalance;
bool at_sharedrel;
@@ -225,7 +227,6 @@ typedef struct WorkerInfoData
TimestampTz wi_launchtime;
bool wi_dobalance;
bool wi_sharedrel;
- double wi_cost_delay;
int wi_cost_limit;
int wi_cost_limit_base;
} WorkerInfoData;
@@ -1756,7 +1757,6 @@ FreeWorkerInfo(int code, Datum arg)
MyWorkerInfo->wi_proc = NULL;
MyWorkerInfo->wi_launchtime = 0;
MyWorkerInfo->wi_dobalance = false;
- MyWorkerInfo->wi_cost_delay = 0;
MyWorkerInfo->wi_cost_limit = 0;
MyWorkerInfo->wi_cost_limit_base = 0;
dlist_push_head(&AutoVacuumShmem->av_freeWorkers,
@@ -1780,11 +1780,37 @@ FreeWorkerInfo(int code, Datum arg)
void
AutoVacuumUpdateDelay(void)
{
- if (MyWorkerInfo)
+ /*
+ * We are using autovacuum-related GUCs to update VacuumCostDelay, so we
+ * only want autovacuum workers and autovacuum launcher to do this.
+ */
+ if (!(am_autovacuum_worker || am_autovacuum_launcher))
+ return;
+
+ if (av_use_table_option_cost_delay)
{
- VacuumCostDelay = MyWorkerInfo->wi_cost_delay;
- VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+ VacuumCostDelay = av_table_option_cost_delay;
}
+ else
+ {
+ VacuumCostDelay = autovacuum_vac_cost_delay >= 0 ?
+ autovacuum_vac_cost_delay : VacuumCostDelay;
+ }
+}
+
+/*
+ * Helper for vacuum_delay_point() to allow workers to read their
+ * wi_cost_limit.
+ */
+void
+AutoVacuumUpdateLimit(void)
+{
+ if (!MyWorkerInfo)
+ return;
+
+ LWLockAcquire(AutovacuumLock, LW_SHARED);
+ VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+ LWLockRelease(AutovacuumLock);
}
/*
@@ -1824,9 +1850,9 @@ autovac_balance_cost(void)
if (worker->wi_proc != NULL &&
worker->wi_dobalance &&
- worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
+ worker->wi_cost_limit_base > 0 && vac_cost_delay > 0)
cost_total +=
- (double) worker->wi_cost_limit_base / worker->wi_cost_delay;
+ (double) worker->wi_cost_limit_base / vac_cost_delay;
}
/* there are no cost limits -- nothing to do */
@@ -1844,7 +1870,7 @@ autovac_balance_cost(void)
if (worker->wi_proc != NULL &&
worker->wi_dobalance &&
- worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
+ worker->wi_cost_limit_base > 0 && vac_cost_delay > 0)
{
int limit = (int)
(cost_avail * worker->wi_cost_limit_base / cost_total);
@@ -1861,11 +1887,10 @@ autovac_balance_cost(void)
}
if (worker->wi_proc != NULL)
- elog(DEBUG2, "autovac_balance_cost(pid=%d db=%u, rel=%u, dobalance=%s cost_limit=%d, cost_limit_base=%d, cost_delay=%g)",
+ elog(DEBUG2, "autovac_balance_cost(pid=%d db=%u, rel=%u, dobalance=%s cost_limit=%d, cost_limit_base=%d)",
worker->wi_proc->pid, worker->wi_dboid, worker->wi_tableoid,
worker->wi_dobalance ? "yes" : "no",
- worker->wi_cost_limit, worker->wi_cost_limit_base,
- worker->wi_cost_delay);
+ worker->wi_cost_limit, worker->wi_cost_limit_base);
}
}
@@ -2326,6 +2351,15 @@ do_autovacuum(void)
ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
+ /*
+ * Autovacuum workers should always update VacuumCostDelay and
+ * VacuumCostLimit in case they were overridden by the reload.
+ */
+ AutoVacuumUpdateDelay();
+ LWLockAcquire(AutovacuumLock, LW_SHARED);
+ VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+ LWLockRelease(AutovacuumLock);
+
/*
* You might be tempted to bail out if we see autovacuum is now
* disabled. Must resist that temptation -- this might be a
@@ -2424,21 +2458,20 @@ do_autovacuum(void)
stdVacuumCostDelay = VacuumCostDelay;
stdVacuumCostLimit = VacuumCostLimit;
+ AutoVacuumUpdateDelay();
+
/* Must hold AutovacuumLock while mucking with cost balance info */
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
/* advertise my cost delay parameters for the balancing algorithm */
MyWorkerInfo->wi_dobalance = tab->at_dobalance;
- MyWorkerInfo->wi_cost_delay = tab->at_vacuum_cost_delay;
MyWorkerInfo->wi_cost_limit = tab->at_vacuum_cost_limit;
MyWorkerInfo->wi_cost_limit_base = tab->at_vacuum_cost_limit;
+ VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
/* do a balance */
autovac_balance_cost();
- /* set the active cost parameters from the result of that */
- AutoVacuumUpdateDelay();
-
/* done */
LWLockRelease(AutovacuumLock);
@@ -2569,6 +2602,11 @@ deleted:
{
ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
+ AutoVacuumUpdateDelay();
+
+ LWLockAcquire(AutovacuumLock, LW_SHARED);
+ VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+ LWLockRelease(AutovacuumLock);
}
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -2771,7 +2809,11 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
/* fetch the relation's relcache entry */
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(classTup))
+ {
+ av_use_table_option_cost_delay = false;
+ av_table_option_cost_delay = 0;
return NULL;
+ }
classForm = (Form_pg_class) GETSTRUCT(classTup);
/*
@@ -2802,7 +2844,6 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
int multixact_freeze_min_age;
int multixact_freeze_table_age;
int vac_cost_limit;
- double vac_cost_delay;
int log_min_duration;
/*
@@ -2812,12 +2853,16 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
* defaults, autovacuum's own first and plain vacuum second.
*/
- /* -1 in autovac setting means use plain vacuum_cost_delay */
- vac_cost_delay = (avopts && avopts->vacuum_cost_delay >= 0)
- ? avopts->vacuum_cost_delay
- : (autovacuum_vac_cost_delay >= 0)
- ? autovacuum_vac_cost_delay
- : VacuumCostDelay;
+ if (avopts && avopts->vacuum_cost_delay >= 0)
+ {
+ av_use_table_option_cost_delay = true;
+ av_table_option_cost_delay = avopts->vacuum_cost_delay;
+ }
+ else
+ {
+ av_use_table_option_cost_delay = false;
+ av_table_option_cost_delay = 0;
+ }
/* 0 or -1 in autovac setting means use plain vacuum_cost_limit */
vac_cost_limit = (avopts && avopts->vacuum_cost_limit > 0)
@@ -2880,7 +2925,6 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
tab->at_params.is_wraparound = wraparound;
tab->at_params.log_min_duration = log_min_duration;
tab->at_vacuum_cost_limit = vac_cost_limit;
- tab->at_vacuum_cost_delay = vac_cost_delay;
tab->at_relname = NULL;
tab->at_nspname = NULL;
tab->at_datname = NULL;
@@ -2893,6 +2937,11 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
!(avopts && (avopts->vacuum_cost_limit > 0 ||
avopts->vacuum_cost_delay > 0));
}
+ else
+ {
+ av_use_table_option_cost_delay = false;
+ av_table_option_cost_delay = 0;
+ }
heap_freetuple(classTup);
return tab;
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index c140371b51..558358911c 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -66,6 +66,8 @@ extern void AutoVacWorkerFailed(void);
/* autovacuum cost-delay balancer */
extern void AutoVacuumUpdateDelay(void);
+extern void AutoVacuumUpdateLimit(void);
+
#ifdef EXEC_BACKEND
extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
extern void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn();
--
2.37.2
