On Thu, Mar 23, 2023 at 8:27 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
> On Thu, Mar 23, 2023 at 2:09 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
> > > And, I was wondering if it was worth trying to split up the part that
> > > reloads the config file and all of the autovacuum stuff. The reloading
> > > of the config file by itself won't actually result in autovacuum workers
> > > having updated cost delays because of them overwriting it with
> > > wi_cost_delay, but it will allow VACUUM to have those updated values.
> >
> > It makes sense to me to have changes for overhauling the rebalance
> > mechanism in a separate patch.
> >
> > Looking back at the original concern you mentioned[1]:
> >
> > speed up long-running vacuum of a large table by
> > decreasing autovacuum_vacuum_cost_delay/vacuum_cost_delay, however the
> > config file is only reloaded between tables (for autovacuum) or after
> > the statement (for explicit vacuum).
> >
> > does it make sense to have autovac_balance_cost() update workers'
> > wi_cost_delay too? Autovacuum launcher already reloads the config file
> > and does the rebalance. So I thought autovac_balance_cost() can update
> > the cost_delay as well, and this might be a minimal change to deal
> > with your concern. This doesn't have the effect for manual VACUUM but
> > since vacuum delay is disabled by default it won't be a big problem.
> > As for manual VACUUMs, we would need to reload the config file in
> > vacuum_delay_point() as the part of your patch does. Overhauling the
> > rebalance mechanism would be another patch to improve it further.
>
> So, we can't do this without acquiring an access shared lock on every
> call to vacuum_delay_point() because cost delay is a double.
>
> I will work on a patchset with separate commits for reloading the config
> file, though (with autovac not benefitting in the first commit).
So, I realized we could actually do as you say and have autovac workers
update their wi_cost_delay and keep the balance changes in a separate
commit. I've done this in attached v8.
Workers take the exclusive lock to update their wi_cost_delay and
wi_cost_limit only when there is a config reload. So, there is one
commit that implements this behavior and a separate commit to revise the
worker rebalancing.
Note that we must have the workers also update wi_cost_limit_base and
then call autovac_balance_cost() when they reload the config file
(instead of waiting for launcher to call autovac_balance_cost()) to
avoid potentially calculating the sleep with a new value of cost delay
and an old value of cost limit.
In the commit which revises the worker rebalancing, I'm still wondering
if wi_dobalance should be an atomic flag -- probably not worth it,
right?
On Fri, Mar 24, 2023 at 1:27 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
> I realized nworkers_for_balance should be initialized to 0 and not 1 --
> 1 is misleading since there are often 0 autovac workers. We just never
> want to use nworkers_for_balance when it is 0. But, workers put a floor
> of 1 on the number when they divide limit/nworkers_for_balance (since
> they know there must be at least one worker right now since they are a
> worker). I thought about whether or not they should call
> autovac_balance_cost() if they find that nworkers_for_balance is 0 when
> updating their own limit, but I'm not sure.
I've gone ahead and updated this. I haven't made the workers call
autovac_balance_cost() if they find that nworkers_for_balance is 0 when
they try and use it when updating their limit because I'm not sure if
this can happen. I would be interested in input here.
I'm also still interested in feedback on the variable name
av_nworkers_for_balance.
> On Fri, Mar 24, 2023 at 1:21 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
> > > > I need another few readthroughs to figure out of VacuumFailsafeActive
> > > > does what
> > > > I think it does, and should be doing, but in general I think this is a
> > > > good
> > > > idea and a patch in good condition close to being committable.
> >
> > Another approach would be to make VacuumCostActive a ternary value:
> > on, off, and never. When we trigger the failsafe mode we switch it to
> > never, meaning that it never becomes active even after reloading the
> > config file. A good point is that we don't need to add a new global
> > variable, but I'm not sure it's better than the current approach.
>
> Hmm, this is interesting. I don't love the word "never" since it kind of
> implies a duration longer than the current table being vacuumed. But we
> could find a different word or just document it well. For clarity, we
> might want to call it failsafe_mode or something.
>
> I wonder if the primary drawback to converting
> LVRelState->failsafe_active to a global VacuumFailsafeActive is just the
> general rule of limiting scope to the minimum needed.
Okay, so I've changed my mind about this. I like having a ternary for
VacuumCostActive and keeping failsafe_active in LVRelState. What I
didn't like was having non-vacuum code have to care about the
distinction between failsafe + inactive and just inactive. To handle
this, I converted VacuumCostActive to VacuumCostInactive since there are
two inactive cases (inactive and failsafe and plain inactive) and only
one active case. Then, I defined VacuumCostInactive as an int but use
enum values for it in vacuum code to distinguish between failsafe +
inactive and just inactive (I call it VACUUM_COST_INACTIVE_AND_LOCKED
and VACUUM_COST_INACTIVE_AND_UNLOCKED). Non-vacuum code only needs to
check if VacuumCostInactive is 0 like if (!VacuumCostInactive). I'm
happy with the result, and I think it employs only well-defined C
behavior.
- Melanie
From 8b9fcf7c10353dcacb4ac16515aad0ce34565566 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Fri, 24 Mar 2023 13:38:20 -0400
Subject: [PATCH v8 3/4] [auto]vacuum reloads config file more often
Previously, VACUUM and autovacuum workers would reload the configuration
file only between vacuuming tables. This precluded user updates to
cost-based delay parameters from taking effect while vacuuming a table.
Check if a reload is pending roughly once per block now, when checking
if we need to delay.
Reviewed-by: Masahiko Sawada <sawada.m...@gmail.com>
Reviewed-by: Daniel Gustafsson <dan...@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_buP5wzsho3qNw5o9_R0pF69FRM5hgCmr-mvXmGXwdA7A%40mail.gmail.com#5e6771d4cdca4db6efc2acec2dce0bc7
---
src/backend/commands/vacuum.c | 72 +++++++++++++++++++++++++----
src/backend/postmaster/autovacuum.c | 30 +++++++++++-
src/include/postmaster/autovacuum.h | 3 +-
3 files changed, 92 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index eb126f2247..cb32078c19 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/pmsignal.h"
@@ -76,6 +77,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;
/*
@@ -314,8 +316,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);
@@ -332,10 +333,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,
@@ -457,7 +458,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;
@@ -475,7 +476,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())
@@ -544,7 +545,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)
{
@@ -568,6 +569,7 @@ vacuum(List *relations, VacuumParams *params,
in_vacuum = false;
VacuumCostInactive = VACUUM_COST_INACTIVE_AND_UNLOCKED;
VacuumCostBalance = 0;
+ analyze_in_outer_xact = false;
}
PG_END_TRY();
@@ -2233,7 +2235,52 @@ vacuum_delay_point(void)
/* Always check for interrupts */
CHECK_FOR_INTERRUPTS();
- if (VacuumCostInactive || InterruptPending)
+ if (InterruptPending ||
+ (VacuumCostInactive && !ConfigReloadPending))
+ return;
+
+ /*
+ * 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. Analyze should
+ * not reload configuration file if it is in an outer transaction, as GUC
+ * values shouldn't be allowed to refer to some uncommitted state (e.g.
+ * database objects created in this transaction).
+ */
+ if (ConfigReloadPending && !analyze_in_outer_xact)
+ {
+ ConfigReloadPending = false;
+ ProcessConfigFile(PGC_SIGHUP);
+
+ /*
+ * Autovacuum workers must restore the correct values of
+ * VacuumCostLimit and VacuumCostDelay in case they were overwritten
+ * by reload.
+ */
+ AutoVacuumUpdateCosts();
+ AutoVacuumOverrideCosts();
+
+ /*
+ * If configuration changes are allowed to impact VacuumCostInactive,
+ * make sure it is updated.
+ */
+ if (VacuumCostInactive == VACUUM_COST_INACTIVE_AND_LOCKED)
+ return;
+
+ if (VacuumCostDelay > 0)
+ VacuumCostInactive = VACUUM_COST_ACTIVE;
+ else
+ {
+ VacuumCostInactive = VACUUM_COST_INACTIVE_AND_UNLOCKED;
+ VacuumCostBalance = 0;
+ }
+ }
+
+ /*
+ * If we disabled cost-based delays after reloading the config file,
+ * return.
+ */
+ if (VacuumCostInactive)
return;
/*
@@ -2266,8 +2313,13 @@ vacuum_delay_point(void)
VacuumCostBalance = 0;
- /* update balance values for workers */
- AutoVacuumUpdateDelay();
+ /*
+ * For autovacuum workers, someone may have called
+ * autovac_balance_cost() since they last updated their
+ * VacuumCostLimit above. Do so again now to ensure they have a
+ * current value.
+ */
+ AutoVacuumOverrideCosts();
/* 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 c0e2e00a7e..8ac14a44c8 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -1778,7 +1778,7 @@ FreeWorkerInfo(int code, Datum arg)
* each a fraction of the total available I/O.
*/
void
-AutoVacuumUpdateDelay(void)
+AutoVacuumOverrideCosts(void)
{
if (MyWorkerInfo)
{
@@ -1787,6 +1787,29 @@ AutoVacuumUpdateDelay(void)
}
}
+/*
+ * Caller must not already hold the AutovacuumLock
+ */
+void
+AutoVacuumUpdateCosts(void)
+{
+ /*
+ * Even though this autovacuum worker may be vacuuming a table with a cost
+ * limit table option and not a cost delay table option, we still don't
+ * refresh the cost delay value.
+ */
+ if (!MyWorkerInfo || !MyWorkerInfo->wi_dobalance)
+ return;
+
+ LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
+ MyWorkerInfo->wi_cost_delay = autovacuum_vac_cost_delay >= 0 ?
+ autovacuum_vac_cost_delay : VacuumCostDelay;
+ MyWorkerInfo->wi_cost_limit_base = autovacuum_vac_cost_limit > 0 ?
+ autovacuum_vac_cost_limit : VacuumCostLimit;
+ autovac_balance_cost();
+ LWLockRelease(AutovacuumLock);
+}
+
/*
* autovac_balance_cost
* Recalculate the cost limit setting for each active worker.
@@ -2320,6 +2343,9 @@ do_autovacuum(void)
/*
* Check for config changes before processing each collected table.
+ * Autovacuum workers must update VacuumCostDelay and VacuumCostLimit
+ * in case they were overridden by the reload. However, we will do
+ * this as soon as we check table options a bit later.
*/
if (ConfigReloadPending)
{
@@ -2437,7 +2463,7 @@ do_autovacuum(void)
autovac_balance_cost();
/* set the active cost parameters from the result of that */
- AutoVacuumUpdateDelay();
+ AutoVacuumOverrideCosts();
/* done */
LWLockRelease(AutovacuumLock);
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index c140371b51..ee48e7123d 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -64,7 +64,8 @@ extern int StartAutoVacWorker(void);
extern void AutoVacWorkerFailed(void);
/* autovacuum cost-delay balancer */
-extern void AutoVacuumUpdateDelay(void);
+extern void AutoVacuumOverrideCosts(void);
+extern void AutoVacuumUpdateCosts(void);
#ifdef EXEC_BACKEND
extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
--
2.37.2
From 9ade10882c6b2daafb846be667de04225046e157 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 25 Mar 2023 11:59:33 -0400
Subject: [PATCH v8 1/4] Zero out VacuumCostBalance
Though it is unlikely to matter, we should zero out VacuumCostBalance
whenever we may be transitioning the state of VacuumCostActive to false.
---
src/backend/commands/vacuum.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index c54360a6a0..7d01df7e48 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -550,6 +550,7 @@ vacuum(List *relations, VacuumParams *params,
{
in_vacuum = false;
VacuumCostActive = false;
+ VacuumCostBalance = 0;
}
PG_END_TRY();
--
2.37.2
From 0a546a538b75ee1a736ff9a09a31412c0b323082 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 25 Mar 2023 14:14:55 -0400
Subject: [PATCH v8 4/4] Improve autovacuum worker cost balancing
Before the prior commit, an autovacuum worker's wi_cost_limit was set
only at the beginning of vacuuming a table, after reloading the config
file. Therefore, at the time that autovac_balance_cost() is called,
workers vacuuming tables with no table options could still have
different values for their wi_cost_limit_base and wi_cost_delay.
Now that the cost parameters can be updated while vacuuming a table,
workers will (within some margin of error) have no reason to have
different values for cost limit and cost delay (in the absence of table
options). This removes the rationale for keeping cost limit and cost
delay in shared memory. Balancing the cost limit requires only the
number of active autovacuum workers vacuuming a table with no cost-based
table options.
---
src/backend/commands/vacuum.c | 18 ++-
src/backend/postmaster/autovacuum.c | 237 ++++++++++++----------------
src/include/postmaster/autovacuum.h | 4 +-
3 files changed, 112 insertions(+), 147 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index cb32078c19..54ad76a729 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2257,12 +2257,13 @@ vacuum_delay_point(void)
* VacuumCostLimit and VacuumCostDelay in case they were overwritten
* by reload.
*/
- AutoVacuumUpdateCosts();
- AutoVacuumOverrideCosts();
+ AutoVacuumUpdateDelay();
+ AutoVacuumUpdateLimit();
/*
* If configuration changes are allowed to impact VacuumCostInactive,
- * make sure it is updated.
+ * make sure it is updated. Autovacuum workers will have already done
+ * this in AutoVacuumUpdateDelay()
*/
if (VacuumCostInactive == VACUUM_COST_INACTIVE_AND_LOCKED)
return;
@@ -2314,12 +2315,13 @@ vacuum_delay_point(void)
VacuumCostBalance = 0;
/*
- * For autovacuum workers, someone may have called
- * autovac_balance_cost() since they last updated their
- * VacuumCostLimit above. Do so again now to ensure they have a
- * current value.
+ * Update limit values for autovacuum workers. We must always do this
+ * in case the autovacuum launcher or another autovacuum worker has
+ * recalculated the number of workers across which we must balance the
+ * limit. This is done by the launcher when launching a new worker and
+ * by workers before vacuuming each table.
*/
- AutoVacuumOverrideCosts();
+ AutoVacuumUpdateLimit();
/* 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 8ac14a44c8..0c20442fb1 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 double av_table_option_cost_delay = -1;
+static int av_table_option_cost_limit = 0;
+
/* Flags set by signal handlers */
static volatile sig_atomic_t got_SIGUSR2 = false;
@@ -189,8 +192,8 @@ typedef struct autovac_table
{
Oid at_relid;
VacuumParams at_params;
- double at_vacuum_cost_delay;
- int at_vacuum_cost_limit;
+ double at_table_option_vac_cost_delay;
+ int at_table_option_vac_cost_limit;
bool at_dobalance;
bool at_sharedrel;
char *at_relname;
@@ -209,7 +212,7 @@ typedef struct autovac_table
* wi_sharedrel flag indicating whether table is marked relisshared
* wi_proc pointer to PGPROC of the running worker, NULL if not started
* wi_launchtime Time at which this worker was launched
- * wi_cost_* Vacuum cost-based delay parameters current in this worker
+ * wi_dobalance Whether this worker should be included in balance calculations
*
* All fields are protected by AutovacuumLock, except for wi_tableoid and
* wi_sharedrel which are protected by AutovacuumScheduleLock (note these
@@ -225,9 +228,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;
typedef struct WorkerInfoData *WorkerInfo;
@@ -273,6 +273,8 @@ typedef struct AutoVacuumWorkItem
* av_startingWorker pointer to WorkerInfo currently being started (cleared by
* the worker itself as soon as it's up and running)
* av_workItems work item array
+ * av_nworkers_for_balance the number of autovacuum workers to use when
+ * calculating the per worker cost limit
*
* This struct is protected by AutovacuumLock, except for av_signal and parts
* of the worker list (see above).
@@ -286,6 +288,7 @@ typedef struct
dlist_head av_runningWorkers;
WorkerInfo av_startingWorker;
AutoVacuumWorkItem av_workItems[NUM_WORKITEMS];
+ pg_atomic_uint32 av_nworkers_for_balance;
} AutoVacuumShmemStruct;
static AutoVacuumShmemStruct *AutoVacuumShmem;
@@ -820,7 +823,7 @@ HandleAutoVacLauncherInterrupts(void)
AutoVacLauncherShutdown();
/* rebalance in case the default cost parameters changed */
- LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
+ LWLockAcquire(AutovacuumLock, LW_SHARED);
autovac_balance_cost();
LWLockRelease(AutovacuumLock);
@@ -1756,9 +1759,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,
&MyWorkerInfo->wi_links);
/* not mine anymore */
@@ -1773,123 +1773,114 @@ FreeWorkerInfo(int code, Datum arg)
}
}
+
/*
- * Update the cost-based delay parameters, so that multiple workers consume
- * each a fraction of the total available I/O.
+ * Update VacuumCostDelay with the correct value for an autovacuum worker,
+ * given the value of other relevant cost-based delay parameters. Autovacuum
+ * workers should call this after every config reload, in case VacuumCostDelay
+ * was overwritten.
*/
void
-AutoVacuumOverrideCosts(void)
+AutoVacuumUpdateDelay(void)
{
- if (MyWorkerInfo)
+ if (!am_autovacuum_worker)
+ return;
+
+ if (av_table_option_cost_delay >= 0)
+ VacuumCostDelay = av_table_option_cost_delay;
+ else if (autovacuum_vac_cost_delay >= 0)
+ VacuumCostDelay = autovacuum_vac_cost_delay;
+
+ /*
+ * If configuration changes are allowed to impact VacuumCostInactive, make
+ * sure it is updated.
+ */
+ if (VacuumCostInactive == VACUUM_COST_INACTIVE_AND_LOCKED)
+ return;
+
+ if (VacuumCostDelay > 0)
+ VacuumCostInactive = VACUUM_COST_ACTIVE;
+ else
{
- VacuumCostDelay = MyWorkerInfo->wi_cost_delay;
- VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+ VacuumCostInactive = VACUUM_COST_INACTIVE_AND_UNLOCKED;
+ VacuumCostBalance = 0;
}
}
+
/*
- * Caller must not already hold the AutovacuumLock
+ * Update VacuumCostLimit with the correct value for an autovacuum worker,
+ * given the value of other relevant cost limit parameters and the number of
+ * workers across which the limit must be balanced. Autovacuum workers must
+ * call this regularly in case av_nworkers_for_balance has been updated by
+ * another worker or by the autovacuum launcher. They also must call this after
+ * every config reload, in case VacuumCostLimit was overwritten.
*/
void
-AutoVacuumUpdateCosts(void)
+AutoVacuumUpdateLimit(void)
{
+ if (!am_autovacuum_worker)
+ return;
+
/*
- * Even though this autovacuum worker may be vacuuming a table with a cost
- * limit table option and not a cost delay table option, we still don't
- * refresh the cost delay value.
+ * note: in cost_limit, zero also means use value from elsewhere, because
+ * zero is not a valid value.
*/
- if (!MyWorkerInfo || !MyWorkerInfo->wi_dobalance)
- return;
+ if (av_table_option_cost_limit > 0)
+ VacuumCostLimit = av_table_option_cost_limit;
+ else
+ {
+ /* There is at least 1 autovac worker (this worker). */
+ int nworkers_for_balance = Max(pg_atomic_read_u32(
+ &AutoVacuumShmem->av_nworkers_for_balance), 1);
- LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
- MyWorkerInfo->wi_cost_delay = autovacuum_vac_cost_delay >= 0 ?
- autovacuum_vac_cost_delay : VacuumCostDelay;
- MyWorkerInfo->wi_cost_limit_base = autovacuum_vac_cost_limit > 0 ?
+ int vac_cost_limit = autovacuum_vac_cost_limit > 0 ?
autovacuum_vac_cost_limit : VacuumCostLimit;
- autovac_balance_cost();
- LWLockRelease(AutovacuumLock);
+
+ int balanced_cost_limit = vac_cost_limit / nworkers_for_balance;
+
+ VacuumCostLimit = Max(Min(balanced_cost_limit, vac_cost_limit), 1);
+ }
}
+
/*
* autovac_balance_cost
- * Recalculate the cost limit setting for each active worker.
+ * Recalculate the number of workers to consider, given table options and
+ * the current number of active workers.
*
- * Caller must hold the AutovacuumLock in exclusive mode.
+ * Caller must hold the AutovacuumLock in at least shared mode.
*/
static void
autovac_balance_cost(void)
{
- /*
- * The idea here is that we ration out I/O equally. The amount of I/O
- * that a worker can consume is determined by cost_limit/cost_delay, so we
- * try to equalize those ratios rather than the raw limit settings.
- *
- * note: in cost_limit, zero also means use value from elsewhere, because
- * zero is not a valid value.
- */
- int vac_cost_limit = (autovacuum_vac_cost_limit > 0 ?
- autovacuum_vac_cost_limit : VacuumCostLimit);
- double vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ?
- autovacuum_vac_cost_delay : VacuumCostDelay);
- double cost_total;
- double cost_avail;
dlist_iter iter;
+ int orig_nworkers_for_balance;
+ int nworkers_for_balance = 0;
- /* not set? nothing to do */
- if (vac_cost_limit <= 0 || vac_cost_delay <= 0)
+ if (autovacuum_vac_cost_delay == 0 ||
+ (autovacuum_vac_cost_delay == -1 && VacuumCostDelay == 0))
return;
- /* calculate the total base cost limit of participating active workers */
- cost_total = 0.0;
- dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers)
- {
- WorkerInfo worker = dlist_container(WorkerInfoData, wi_links, iter.cur);
-
- if (worker->wi_proc != NULL &&
- worker->wi_dobalance &&
- worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
- cost_total +=
- (double) worker->wi_cost_limit_base / worker->wi_cost_delay;
- }
-
- /* there are no cost limits -- nothing to do */
- if (cost_total <= 0)
+ if (autovacuum_vac_cost_limit <= 0 && VacuumCostLimit <= 0)
return;
- /*
- * Adjust cost limit of each active worker to balance the total of cost
- * limit to autovacuum_vacuum_cost_limit.
- */
- cost_avail = (double) vac_cost_limit / vac_cost_delay;
+ orig_nworkers_for_balance =
+ pg_atomic_read_u32(&AutoVacuumShmem->av_nworkers_for_balance);
+
dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers)
{
WorkerInfo worker = dlist_container(WorkerInfoData, wi_links, iter.cur);
- if (worker->wi_proc != NULL &&
- worker->wi_dobalance &&
- worker->wi_cost_limit_base > 0 && worker->wi_cost_delay > 0)
- {
- int limit = (int)
- (cost_avail * worker->wi_cost_limit_base / cost_total);
-
- /*
- * We put a lower bound of 1 on the cost_limit, to avoid division-
- * by-zero in the vacuum code. Also, in case of roundoff trouble
- * in these calculations, let's be sure we don't ever set
- * cost_limit to more than the base value.
- */
- worker->wi_cost_limit = Max(Min(limit,
- worker->wi_cost_limit_base),
- 1);
- }
+ if (worker->wi_proc == NULL || !worker->wi_dobalance)
+ continue;
- 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)",
- 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);
+ nworkers_for_balance++;
}
+
+ if (nworkers_for_balance != orig_nworkers_for_balance)
+ pg_atomic_write_u32(&AutoVacuumShmem->av_nworkers_for_balance,
+ nworkers_for_balance);
}
/*
@@ -2335,8 +2326,6 @@ do_autovacuum(void)
autovac_table *tab;
bool isshared;
bool skipit;
- double stdVacuumCostDelay;
- int stdVacuumCostLimit;
dlist_iter iter;
CHECK_FOR_INTERRUPTS();
@@ -2442,32 +2431,18 @@ do_autovacuum(void)
continue;
}
- /*
- * Remember the prevailing values of the vacuum cost GUCs. We have to
- * restore these at the bottom of the loop, else we'll compute wrong
- * values in the next iteration of autovac_balance_cost().
- */
- stdVacuumCostDelay = VacuumCostDelay;
- stdVacuumCostLimit = VacuumCostLimit;
+ av_table_option_cost_limit = tab->at_table_option_vac_cost_limit;
+ av_table_option_cost_delay = tab->at_table_option_vac_cost_delay;
/* 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;
-
- /* do a balance */
autovac_balance_cost();
-
- /* set the active cost parameters from the result of that */
- AutoVacuumOverrideCosts();
-
- /* done */
LWLockRelease(AutovacuumLock);
+ AutoVacuumUpdateDelay();
+ AutoVacuumUpdateLimit();
+
/* clean up memory before each iteration */
MemoryContextResetAndDeleteChildren(PortalContext);
@@ -2551,19 +2526,15 @@ deleted:
/*
* Remove my info from shared memory. We could, but intentionally
- * don't, clear wi_cost_limit and friends --- this is on the
- * assumption that we probably have more to do with similar cost
- * settings, so we don't want to give up our share of I/O for a very
- * short interval and thereby thrash the global balance.
+ * don't, set wi_dobalance to false on the assumption that we are more
+ * likely than not to vacuum a table with no table options next, so we
+ * don't want to give up our share of I/O for a very short interval
+ * and thereby thrash the global balance.
*/
LWLockAcquire(AutovacuumScheduleLock, LW_EXCLUSIVE);
MyWorkerInfo->wi_tableoid = InvalidOid;
MyWorkerInfo->wi_sharedrel = false;
LWLockRelease(AutovacuumScheduleLock);
-
- /* restore vacuum cost GUCs for the next iteration */
- VacuumCostDelay = stdVacuumCostDelay;
- VacuumCostLimit = stdVacuumCostLimit;
}
/*
@@ -2595,6 +2566,8 @@ deleted:
{
ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
+ AutoVacuumUpdateDelay();
+ AutoVacuumUpdateLimit();
}
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -2827,8 +2800,6 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
int freeze_table_age;
int multixact_freeze_min_age;
int multixact_freeze_table_age;
- int vac_cost_limit;
- double vac_cost_delay;
int log_min_duration;
/*
@@ -2838,20 +2809,6 @@ 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;
-
- /* 0 or -1 in autovac setting means use plain vacuum_cost_limit */
- vac_cost_limit = (avopts && avopts->vacuum_cost_limit > 0)
- ? avopts->vacuum_cost_limit
- : (autovacuum_vac_cost_limit > 0)
- ? autovacuum_vac_cost_limit
- : VacuumCostLimit;
-
/* -1 in autovac setting means use log_autovacuum_min_duration */
log_min_duration = (avopts && avopts->log_min_duration >= 0)
? avopts->log_min_duration
@@ -2907,8 +2864,10 @@ 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;
- tab->at_vacuum_cost_limit = vac_cost_limit;
- tab->at_vacuum_cost_delay = vac_cost_delay;
+ tab->at_table_option_vac_cost_limit = avopts ?
+ avopts->vacuum_cost_limit : 0;
+ tab->at_table_option_vac_cost_delay = avopts ?
+ avopts->vacuum_cost_delay : -1;
tab->at_relname = NULL;
tab->at_nspname = NULL;
tab->at_datname = NULL;
@@ -3400,10 +3359,14 @@ AutoVacuumShmemInit(void)
worker = (WorkerInfo) ((char *) AutoVacuumShmem +
MAXALIGN(sizeof(AutoVacuumShmemStruct)));
+
/* initialize the WorkerInfo free list */
for (i = 0; i < autovacuum_max_workers; i++)
dlist_push_head(&AutoVacuumShmem->av_freeWorkers,
&worker[i].wi_links);
+
+ pg_atomic_init_u32(&AutoVacuumShmem->av_nworkers_for_balance, 0);
+
}
else
Assert(found);
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index ee48e7123d..7b462866c9 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -64,8 +64,8 @@ extern int StartAutoVacWorker(void);
extern void AutoVacWorkerFailed(void);
/* autovacuum cost-delay balancer */
-extern void AutoVacuumOverrideCosts(void);
-extern void AutoVacuumUpdateCosts(void);
+extern void AutoVacuumUpdateDelay(void);
+extern void AutoVacuumUpdateLimit(void);
#ifdef EXEC_BACKEND
extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
--
2.37.2
From 3a9492b1f3eaacafc730aea0d53085046886ecad Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 25 Mar 2023 12:05:18 -0400
Subject: [PATCH v8 2/4] Make VacuumCostActive failsafe-aware
While vacuuming a table in failsafe mode, VacuumCostActive should not be
re-enabled. This currently isn't a problem because vacuum cost
parameters are only refreshed in between vacuuming tables and failsafe
status is reset for every table. In preparation for allowing vacuum cost
parameters to be updated more frequently, make vacuum cost status more
expressive.
VacuumCostActive is now VacuumCostInactive, as it can only be active in
one way but it can be inactive in two ways. If performing a failsafe
vacuum, the vacuum cost status cannot be enabled and is effectively
"locked". If performing a non-failsafe vacuum, the vacuum cost status
may be active or inactive. To express this, VacuumCostInactive can be
one of three statuses: VACUUM_COST_INACTIVE_AND_LOCKED,
VACUUM_COST_ACTIVE_AND_LOCKED, and VACUUM_COST_ACTIVE.
VacuumCostInactive is defined as an integer because we do not want
non-vacuum code concerning itself with the distinction between the three
statuses -- only with whether or not VacuumCostInactive == 0 or not.
---
src/backend/access/heap/vacuumlazy.c | 2 +-
src/backend/commands/vacuum.c | 23 ++++++++++++++++++++---
src/backend/commands/vacuumparallel.c | 8 ++++++--
src/backend/storage/buffer/bufmgr.c | 8 ++++----
src/backend/utils/init/globals.c | 2 +-
src/include/commands/vacuum.h | 8 ++++++++
src/include/miscadmin.h | 3 +--
7 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8f14cf85f3..040a4e931b 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2637,7 +2637,7 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
"You might also need to consider other ways for VACUUM to keep up with the allocation of transaction IDs.")));
/* Stop applying cost limits from this point on */
- VacuumCostActive = false;
+ VacuumCostInactive = VACUUM_COST_INACTIVE_AND_LOCKED;
VacuumCostBalance = 0;
return true;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7d01df7e48..eb126f2247 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -491,7 +491,6 @@ vacuum(List *relations, VacuumParams *params,
ListCell *cur;
in_vacuum = true;
- VacuumCostActive = (VacuumCostDelay > 0);
VacuumCostBalance = 0;
VacuumPageHit = 0;
VacuumPageMiss = 0;
@@ -507,6 +506,24 @@ vacuum(List *relations, VacuumParams *params,
{
VacuumRelation *vrel = lfirst_node(VacuumRelation, cur);
+ /*
+ * failsafe_active is reset per relation, so we must be sure that
+ * VacuumCostInactive is set to either VACUUM_COST_INACTIVE or
+ * VACUUM_COST_INACTIVE_AND_UNLOCKED in between vacuuming
+ * relations.
+ */
+ VacuumCostInactive = VacuumCostDelay > 0 ? VACUUM_COST_ACTIVE :
+ VACUUM_COST_INACTIVE_AND_UNLOCKED;
+
+ /*
+ * We should not have transitioned VacuumCostInactive from
+ * VACUUM_COST_ACTIVE to VACUUM_COST_INACTIVE_AND_UNLOCKED above,
+ * as that should have happened when we changed the value of
+ * VacuumCostDelay.
+ */
+ Assert(VacuumCostInactive == VACUUM_COST_ACTIVE ||
+ VacuumCostBalance == 0);
+
if (params->options & VACOPT_VACUUM)
{
if (!vacuum_rel(vrel->oid, vrel->relation, params, false))
@@ -549,7 +566,7 @@ vacuum(List *relations, VacuumParams *params,
PG_FINALLY();
{
in_vacuum = false;
- VacuumCostActive = false;
+ VacuumCostInactive = VACUUM_COST_INACTIVE_AND_UNLOCKED;
VacuumCostBalance = 0;
}
PG_END_TRY();
@@ -2216,7 +2233,7 @@ vacuum_delay_point(void)
/* Always check for interrupts */
CHECK_FOR_INTERRUPTS();
- if (!VacuumCostActive || InterruptPending)
+ if (VacuumCostInactive || InterruptPending)
return;
/*
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index bcd40c80a1..266bf6bb4c 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -989,8 +989,12 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
PARALLEL_VACUUM_KEY_DEAD_ITEMS,
false);
- /* Set cost-based vacuum delay */
- VacuumCostActive = (VacuumCostDelay > 0);
+ /*
+ * Set cost-based vacuum delay Parallel vacuum workers will not execute
+ * failsafe VACUUM.
+ */
+ VacuumCostInactive = VacuumCostDelay > 0 ? VACUUM_COST_ACTIVE :
+ VACUUM_COST_INACTIVE_AND_UNLOCKED;
VacuumCostBalance = 0;
VacuumPageHit = 0;
VacuumPageMiss = 0;
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 95212a3941..6d3dd26fc7 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -893,7 +893,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
*hit = true;
VacuumPageHit++;
- if (VacuumCostActive)
+ if (!VacuumCostInactive)
VacuumCostBalance += VacuumCostPageHit;
TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
@@ -1098,7 +1098,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
}
VacuumPageMiss++;
- if (VacuumCostActive)
+ if (!VacuumCostInactive)
VacuumCostBalance += VacuumCostPageMiss;
TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
@@ -1672,7 +1672,7 @@ MarkBufferDirty(Buffer buffer)
{
VacuumPageDirty++;
pgBufferUsage.shared_blks_dirtied++;
- if (VacuumCostActive)
+ if (!VacuumCostInactive)
VacuumCostBalance += VacuumCostPageDirty;
}
}
@@ -4199,7 +4199,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
{
VacuumPageDirty++;
pgBufferUsage.shared_blks_dirtied++;
- if (VacuumCostActive)
+ if (!VacuumCostInactive)
VacuumCostBalance += VacuumCostPageDirty;
}
}
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..608ebb9182 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -150,4 +150,4 @@ int64 VacuumPageMiss = 0;
int64 VacuumPageDirty = 0;
int VacuumCostBalance = 0; /* working state for vacuum */
-bool VacuumCostActive = false;
+int VacuumCostInactive = 1;
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index bdfd96cfec..5c3e250b06 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -302,6 +302,14 @@ extern PGDLLIMPORT int vacuum_failsafe_age;
extern PGDLLIMPORT int vacuum_multixact_failsafe_age;
/* Variables for cost-based parallel vacuum */
+
+typedef enum VacuumCostStatus
+{
+ VACUUM_COST_INACTIVE_AND_LOCKED = -1,
+ VACUUM_COST_ACTIVE = 0,
+ VACUUM_COST_INACTIVE_AND_UNLOCKED = 1,
+} VacuumCostStatus;
+
extern PGDLLIMPORT pg_atomic_uint32 *VacuumSharedCostBalance;
extern PGDLLIMPORT pg_atomic_uint32 *VacuumActiveNWorkers;
extern PGDLLIMPORT int VacuumCostBalanceLocal;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1..33e22733ae 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -274,8 +274,7 @@ extern PGDLLIMPORT int64 VacuumPageMiss;
extern PGDLLIMPORT int64 VacuumPageDirty;
extern PGDLLIMPORT int VacuumCostBalance;
-extern PGDLLIMPORT bool VacuumCostActive;
-
+extern PGDLLIMPORT int VacuumCostInactive;
/* in tcop/postgres.c */
--
2.37.2
