On Mon, Apr 3, 2023 at 3:08 PM Andres Freund <and...@anarazel.de> wrote: > On 2023-04-03 14:43:14 -0400, Tom Lane wrote: > > Melanie Plageman <melanieplage...@gmail.com> writes: > > > v13 attached with requested updates. > > > > I'm afraid I'd not been paying any attention to this discussion, > > but better late than never. I'm okay with letting autovacuum > > processes reload config files more often than now. However, > > I object to allowing ProcessConfigFile to be called from within > > commands in a normal user backend. The existing semantics are > > that user backends respond to SIGHUP only at the start of processing > > a user command, and I'm uncomfortable with suddenly deciding that > > that can work differently if the command happens to be VACUUM. > > It seems unprincipled and perhaps actively unsafe. > > I think it should be ok in commands like VACUUM that already internally start > their own transactions, and thus require to be run outside of a transaction > and at the toplevel. I share your concerns about allowing config reload in > arbitrary places. While we might want to go there, it would require a lot more > analysis.
As an alternative for your consideration, attached v14 set implements the config file reload for autovacuum only (in 0003) and then enables it for VACUUM and ANALYZE not in a nested transaction command (in 0004). Previously I had the commits in the reverse order for ease of review (to separate changes to worker limit balancing logic from config reload code). - Melanie
From 1781dd7174d5d6eaaeb4bd02029212f3c23d4dbe Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Sat, 25 Mar 2023 14:14:55 -0400 Subject: [PATCH v14 3/4] Autovacuum refreshes cost-based delay params more often Allow autovacuum to reload the config file more often so that cost-based delay parameters can take effect while VACUUMing a relation. Previously autovacuum workers only reloaded the config file once per relation vacuumed, so config changes could not take effect until beginning to vacuum the next table. Now, check if a reload is pending roughly once per block, when checking if we need to delay. In order for autovacuum workers to safely update their own cost delay and cost limit parameters without impacting performance, we had to rethink when and how these values were accessed. Previously, 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. Reviewed-by: Masahiko Sawada <sawada.m...@gmail.com> Reviewed-by: Kyotaro Horiguchi <horikyota....@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com --- src/backend/commands/vacuum.c | 44 ++++- src/backend/postmaster/autovacuum.c | 253 +++++++++++++++------------- src/include/commands/vacuum.h | 1 + 3 files changed, 180 insertions(+), 118 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 96df5e2920..a51a3f78a0 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" @@ -510,9 +511,9 @@ vacuum(List *relations, VacuumParams *params, { ListCell *cur; - VacuumUpdateCosts(); in_vacuum = true; - VacuumCostActive = (VacuumCostDelay > 0); + VacuumFailsafeActive = false; + VacuumUpdateCosts(); VacuumCostBalance = 0; VacuumPageHit = 0; VacuumPageMiss = 0; @@ -566,12 +567,20 @@ vacuum(List *relations, VacuumParams *params, CommandCounterIncrement(); } } + + /* + * Ensure VacuumFailsafeActive has been reset before vacuuming the + * next relation relation. + */ + VacuumFailsafeActive = false; } } PG_FINALLY(); { in_vacuum = false; VacuumCostActive = false; + VacuumFailsafeActive = false; + VacuumCostBalance = 0; } PG_END_TRY(); @@ -2238,7 +2247,27 @@ vacuum_delay_point(void) /* Always check for interrupts */ CHECK_FOR_INTERRUPTS(); - if (!VacuumCostActive || InterruptPending) + if (InterruptPending || + (!VacuumCostActive && !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. + */ + if (ConfigReloadPending && IsAutoVacuumWorkerProcess()) + { + ConfigReloadPending = false; + ProcessConfigFile(PGC_SIGHUP); + VacuumUpdateCosts(); + } + + /* + * If we disabled cost-based delays after reloading the config file, + * return. + */ + if (!VacuumCostActive) return; /* @@ -2271,7 +2300,14 @@ vacuum_delay_point(void) VacuumCostBalance = 0; - VacuumUpdateCosts(); + /* + * Balance and 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. + */ + 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 e0c568fdaf..8cb9bee4eb 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_relopt_cost_delay = -1; +static int av_relopt_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_relopt_vac_cost_delay; + int at_relopt_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 @@ -223,11 +226,8 @@ typedef struct WorkerInfoData Oid wi_tableoid; PGPROC *wi_proc; TimestampTz wi_launchtime; - bool wi_dobalance; + pg_atomic_flag 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_nworkersForBalance 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_nworkersForBalance; } AutoVacuumShmemStruct; static AutoVacuumShmemStruct *AutoVacuumShmem; @@ -319,7 +322,7 @@ static void launch_worker(TimestampTz now); static List *get_database_list(void); static void rebuild_database_list(Oid newdb); static int db_comparator(const void *a, const void *b); -static void autovac_balance_cost(void); +static void autovac_recalculate_workers_for_balance(void); static void do_autovacuum(void); static void FreeWorkerInfo(int code, Datum arg); @@ -670,7 +673,7 @@ AutoVacLauncherMain(int argc, char *argv[]) { LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE); AutoVacuumShmem->av_signal[AutoVacRebalance] = false; - autovac_balance_cost(); + autovac_recalculate_workers_for_balance(); LWLockRelease(AutovacuumLock); } @@ -820,8 +823,8 @@ HandleAutoVacLauncherInterrupts(void) AutoVacLauncherShutdown(); /* rebalance in case the default cost parameters changed */ - LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE); - autovac_balance_cost(); + LWLockAcquire(AutovacuumLock, LW_SHARED); + autovac_recalculate_workers_for_balance(); LWLockRelease(AutovacuumLock); /* rebuild the list in case the naptime changed */ @@ -1755,10 +1758,7 @@ FreeWorkerInfo(int code, Datum arg) MyWorkerInfo->wi_sharedrel = false; 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; + pg_atomic_clear_flag(&MyWorkerInfo->wi_dobalance); dlist_push_head(&AutoVacuumShmem->av_freeWorkers, &MyWorkerInfo->wi_links); /* not mine anymore */ @@ -1787,97 +1787,119 @@ VacuumUpdateCosts(void) if (am_autovacuum_worker) { - VacuumCostDelay = MyWorkerInfo->wi_cost_delay; - VacuumCostLimit = MyWorkerInfo->wi_cost_limit; + if (av_relopt_cost_delay >= 0) + VacuumCostDelay = av_relopt_cost_delay; + else if (autovacuum_vac_cost_delay >= 0) + VacuumCostDelay = autovacuum_vac_cost_delay; + else + /* fall back to vacuum_cost_delay */ + VacuumCostDelay = vacuum_cost_delay; + + AutoVacuumUpdateLimit(); } else { /* Must be explicit VACUUM or ANALYZE */ - VacuumCostLimit = vacuum_cost_limit; VacuumCostDelay = vacuum_cost_delay; + VacuumCostLimit = vacuum_cost_limit; + } + + /* + * If configuration changes are allowed to impact VacuumCostActive, make + * sure it is updated. + */ + if (VacuumFailsafeActive) + { + Assert(!VacuumCostActive); + return; + } + + if (VacuumCostDelay > 0) + VacuumCostActive = true; + else + { + VacuumCostActive = false; + VacuumCostBalance = 0; } } /* - * autovac_balance_cost - * Recalculate the cost limit setting for each active worker. - * - * Caller must hold the AutovacuumLock in exclusive mode. - */ -static void -autovac_balance_cost(void) +* 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 must also call it after a config reload. +*/ +void +AutoVacuumUpdateLimit(void) { + if (!am_autovacuum_worker) + return; + /* - * 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 : vacuum_cost_limit); - double vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ? - autovacuum_vac_cost_delay : vacuum_cost_delay); - double cost_total; - double cost_avail; - dlist_iter iter; - - /* not set? nothing to do */ - if (vac_cost_limit <= 0 || vac_cost_delay <= 0) - return; - /* calculate the total base cost limit of participating active workers */ - cost_total = 0.0; - dlist_foreach(iter, &AutoVacuumShmem->av_runningWorkers) + if (av_relopt_cost_limit > 0) + VacuumCostLimit = av_relopt_cost_limit; + else { - WorkerInfo worker = dlist_container(WorkerInfoData, wi_links, iter.cur); + int nworkers_for_balance; + + if (autovacuum_vac_cost_limit > 0) + VacuumCostLimit = autovacuum_vac_cost_limit; + else + VacuumCostLimit = vacuum_cost_limit; + + /* Only balance limit if no table options specified */ + if (pg_atomic_unlocked_test_flag(&MyWorkerInfo->wi_dobalance)) + return; - 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; + Assert(VacuumCostLimit > 0); + + nworkers_for_balance = pg_atomic_read_u32( + &AutoVacuumShmem->av_nworkersForBalance); + + /* There is at least 1 autovac worker (this worker). */ + Assert(nworkers_for_balance > 0); + + VacuumCostLimit = Max(VacuumCostLimit / nworkers_for_balance, 1); } +} - /* there are no cost limits -- nothing to do */ - if (cost_total <= 0) - return; +/* + * autovac_recalculate_workers_for_balance + * Recalculate the number of workers to consider, given table options and + * the current number of active workers. + * + * Caller must hold the AutovacuumLock in at least shared mode to access + * worker->wi_proc. + */ +static void +autovac_recalculate_workers_for_balance(void) +{ + dlist_iter iter; + int orig_nworkers_for_balance; + int nworkers_for_balance = 0; + + orig_nworkers_for_balance = + pg_atomic_read_u32(&AutoVacuumShmem->av_nworkersForBalance); - /* - * 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; 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 || + pg_atomic_unlocked_test_flag(&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_nworkersForBalance, + nworkers_for_balance); } /* @@ -2425,23 +2447,31 @@ do_autovacuum(void) continue; } - /* 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; + /* + * Save the cost-related table options in global variables for + * reference when updating VacuumCostLimit and VacuumCostDelay during + * vacuuming this table. + */ + av_relopt_cost_limit = tab->at_relopt_vac_cost_limit; + av_relopt_cost_delay = tab->at_relopt_vac_cost_delay; - /* do a balance */ - autovac_balance_cost(); + if (tab->at_dobalance) + pg_atomic_test_set_flag(&MyWorkerInfo->wi_dobalance); + else + pg_atomic_clear_flag(&MyWorkerInfo->wi_dobalance); - /* set the active cost parameters from the result of that */ + LWLockAcquire(AutovacuumLock, LW_SHARED); + autovac_recalculate_workers_for_balance(); + LWLockRelease(AutovacuumLock); + + /* + * We wait until this point to update cost delay and cost limit + * values, even though we reloaded the configuration file above, so + * that we can take into account the cost-related table options. + */ VacuumUpdateCosts(); - /* done */ - LWLockRelease(AutovacuumLock); /* clean up memory before each iteration */ MemoryContextResetAndDeleteChildren(PortalContext); @@ -2526,10 +2556,10 @@ 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, unset wi_dobalance 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; @@ -2566,6 +2596,7 @@ deleted: { ConfigReloadPending = false; ProcessConfigFile(PGC_SIGHUP); + VacuumUpdateCosts(); } LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE); @@ -2801,8 +2832,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; /* @@ -2812,20 +2841,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 - : vacuum_cost_delay; - - /* 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 - : vacuum_cost_limit; - /* -1 in autovac setting means use log_autovacuum_min_duration */ log_min_duration = (avopts && avopts->log_min_duration >= 0) ? avopts->log_min_duration @@ -2881,8 +2896,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_relopt_vac_cost_limit = avopts ? + avopts->vacuum_cost_limit : 0; + tab->at_relopt_vac_cost_delay = avopts ? + avopts->vacuum_cost_delay : -1; tab->at_relname = NULL; tab->at_nspname = NULL; tab->at_datname = NULL; @@ -3374,10 +3391,18 @@ 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_flag(&worker[i].wi_dobalance); + } + + pg_atomic_init_u32(&AutoVacuumShmem->av_nworkersForBalance, 0); + } else Assert(found); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index a62dd2e781..6b286037ca 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -351,6 +351,7 @@ extern IndexBulkDeleteResult *vac_cleanup_one_index(IndexVacuumInfo *ivinfo, extern Size vac_max_items_to_alloc_size(int max_items); /* In postmaster/autovacuum.c */ +extern void AutoVacuumUpdateLimit(void); extern void VacuumUpdateCosts(void); /* in commands/vacuumparallel.c */ -- 2.37.2
From 4cd9317cdec9c30e97bee30b194c75455c0afc99 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Mon, 3 Apr 2023 17:48:22 -0400 Subject: [PATCH v14 4/4] VACUUM reloads config file more often A previous commit allowed autovacuum workers to reload the configuration file more often. Now, enable this behavior for VACUUM and ANALYZE, when not nested in a transaction command. Note that, for now, this only applies to the primary VACUUM stage and not to parallel vacuum workers vacuuming indexes. --- src/backend/commands/vacuum.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index a51a3f78a0..3c426ed501 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -84,6 +84,7 @@ int vacuum_cost_limit; */ int VacuumCostLimit = 0; double VacuumCostDelay = -1; +static bool vacuum_can_reload_config = false; /* * VacuumFailsafeActive is a defined as a global so that we can determine @@ -355,6 +356,8 @@ vacuum(List *relations, VacuumParams *params, else in_outer_xact = IsInTransactionBlock(isTopLevel); + vacuum_can_reload_config = !in_outer_xact; + /* * Check for and disallow recursive calls. This could happen when VACUUM * FULL or ANALYZE calls a hostile index expression that itself calls @@ -581,6 +584,7 @@ vacuum(List *relations, VacuumParams *params, VacuumCostActive = false; VacuumFailsafeActive = false; VacuumCostBalance = 0; + vacuum_can_reload_config = false; } PG_END_TRY(); @@ -2253,10 +2257,12 @@ vacuum_delay_point(void) /* * 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. + * [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 we + * currently only allow configuration reload when in top-level statements. */ - if (ConfigReloadPending && IsAutoVacuumWorkerProcess()) + if (ConfigReloadPending && vacuum_can_reload_config) { ConfigReloadPending = false; ProcessConfigFile(PGC_SIGHUP); -- 2.37.2
From 7ee209bee5c096601ed3ad76ee4ab9ced340e3d3 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Mon, 3 Apr 2023 11:22:18 -0400 Subject: [PATCH v14 2/4] Separate vacuum cost variables from gucs Vacuum code run both by autovacuum workers and a backend doing VACUUM/ANALYZE previously used VacuumCostLimit and VacuumCostDelay which were the global variables for the gucs vacuum_cost_limit and vacuum_cost_delay. Autovacuum workers needed to override these variables with their own values, derived from autovacuum_vacuum_cost_limit and autovacuum_vacuum_cost_delay and worker cost limit balancing logic. This led to confusing code which, in some cases, both derived and set a new value of VacuumCostLimit from VacuumCostLimit. In preparation for refreshing these guc values more often, separate these variables from the gucs themselves and add a function to update the global variables using the gucs and existing logic. Reviewed-by: Masahiko Sawada <sawada.m...@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com --- src/backend/commands/vacuum.c | 16 ++++++++-- src/backend/commands/vacuumparallel.c | 3 +- src/backend/postmaster/autovacuum.c | 43 +++++++++++++-------------- src/backend/utils/init/globals.c | 2 -- src/backend/utils/misc/guc_tables.c | 4 +-- src/include/commands/vacuum.h | 7 +++++ src/include/miscadmin.h | 2 -- src/include/postmaster/autovacuum.h | 3 -- 8 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 9724fbce46..96df5e2920 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -71,6 +71,18 @@ int vacuum_multixact_freeze_min_age; int vacuum_multixact_freeze_table_age; int vacuum_failsafe_age; int vacuum_multixact_failsafe_age; +double vacuum_cost_delay; +int vacuum_cost_limit; + +/* + * Variables for cost-based vacuum delay. The defaults differ between + * autovacuum and vacuum. These should be overridden with the appropriate GUC + * value in vacuum code. + * TODO: should VacuumCostLimit and VacuumCostDelay be initialized to valid or + * invalid values? + */ +int VacuumCostLimit = 0; +double VacuumCostDelay = -1; /* * VacuumFailsafeActive is a defined as a global so that we can determine @@ -498,6 +510,7 @@ vacuum(List *relations, VacuumParams *params, { ListCell *cur; + VacuumUpdateCosts(); in_vacuum = true; VacuumCostActive = (VacuumCostDelay > 0); VacuumCostBalance = 0; @@ -2258,8 +2271,7 @@ vacuum_delay_point(void) VacuumCostBalance = 0; - /* update balance values for workers */ - AutoVacuumUpdateDelay(); + VacuumUpdateCosts(); /* Might have gotten an interrupt while sleeping */ CHECK_FOR_INTERRUPTS(); diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index 563117a8f6..d346838cfc 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -995,8 +995,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) false); /* Set cost-based vacuum delay */ - VacuumCostActive = (VacuumCostDelay > 0); VacuumCostBalance = 0; + VacuumUpdateCosts(); + VacuumCostActive = (VacuumCostDelay > 0); VacuumPageHit = 0; VacuumPageMiss = 0; VacuumPageDirty = 0; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 585d28148c..e0c568fdaf 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1774,17 +1774,28 @@ 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 vacuum cost-based delay-related parameters for autovacuum workers and + * backends executing VACUUM or ANALYZE using the value of relevant gucs and + * global state. This must be called during setup for vacuum and after every + * config reload to ensure up-to-date values. */ void -AutoVacuumUpdateDelay(void) +VacuumUpdateCosts(void) { - if (MyWorkerInfo) + if (am_autovacuum_launcher) + return; + + if (am_autovacuum_worker) { VacuumCostDelay = MyWorkerInfo->wi_cost_delay; VacuumCostLimit = MyWorkerInfo->wi_cost_limit; } + else + { + /* Must be explicit VACUUM or ANALYZE */ + VacuumCostLimit = vacuum_cost_limit; + VacuumCostDelay = vacuum_cost_delay; + } } /* @@ -1805,9 +1816,9 @@ autovac_balance_cost(void) * zero is not a valid value. */ int vac_cost_limit = (autovacuum_vac_cost_limit > 0 ? - autovacuum_vac_cost_limit : VacuumCostLimit); + autovacuum_vac_cost_limit : vacuum_cost_limit); double vac_cost_delay = (autovacuum_vac_cost_delay >= 0 ? - autovacuum_vac_cost_delay : VacuumCostDelay); + autovacuum_vac_cost_delay : vacuum_cost_delay); double cost_total; double cost_avail; dlist_iter iter; @@ -2312,8 +2323,6 @@ do_autovacuum(void) autovac_table *tab; bool isshared; bool skipit; - double stdVacuumCostDelay; - int stdVacuumCostLimit; dlist_iter iter; CHECK_FOR_INTERRUPTS(); @@ -2416,14 +2425,6 @@ 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; - /* Must hold AutovacuumLock while mucking with cost balance info */ LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE); @@ -2437,7 +2438,7 @@ do_autovacuum(void) autovac_balance_cost(); /* set the active cost parameters from the result of that */ - AutoVacuumUpdateDelay(); + VacuumUpdateCosts(); /* done */ LWLockRelease(AutovacuumLock); @@ -2534,10 +2535,6 @@ deleted: MyWorkerInfo->wi_tableoid = InvalidOid; MyWorkerInfo->wi_sharedrel = false; LWLockRelease(AutovacuumScheduleLock); - - /* restore vacuum cost GUCs for the next iteration */ - VacuumCostDelay = stdVacuumCostDelay; - VacuumCostLimit = stdVacuumCostLimit; } /* @@ -2820,14 +2817,14 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, ? avopts->vacuum_cost_delay : (autovacuum_vac_cost_delay >= 0) ? autovacuum_vac_cost_delay - : VacuumCostDelay; + : vacuum_cost_delay; /* 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; + : vacuum_cost_limit; /* -1 in autovac setting means use log_autovacuum_min_duration */ log_min_duration = (avopts && avopts->log_min_duration >= 0) diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 1b1d814254..8e5b065e8f 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -142,8 +142,6 @@ int MaxBackends = 0; int VacuumCostPageHit = 1; /* GUC parameters for vacuum */ int VacuumCostPageMiss = 2; int VacuumCostPageDirty = 20; -int VacuumCostLimit = 200; -double VacuumCostDelay = 0; int64 VacuumPageHit = 0; int64 VacuumPageMiss = 0; diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 8062589efd..77db1a146c 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2409,7 +2409,7 @@ struct config_int ConfigureNamesInt[] = gettext_noop("Vacuum cost amount available before napping."), NULL }, - &VacuumCostLimit, + &vacuum_cost_limit, 200, 1, 10000, NULL, NULL, NULL }, @@ -3701,7 +3701,7 @@ struct config_real ConfigureNamesReal[] = NULL, GUC_UNIT_MS }, - &VacuumCostDelay, + &vacuum_cost_delay, 0, 0, 100, NULL, NULL, NULL }, diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 7b8ee21788..a62dd2e781 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -300,6 +300,8 @@ extern PGDLLIMPORT int vacuum_multixact_freeze_min_age; extern PGDLLIMPORT int vacuum_multixact_freeze_table_age; extern PGDLLIMPORT int vacuum_failsafe_age; extern PGDLLIMPORT int vacuum_multixact_failsafe_age; +extern PGDLLIMPORT double vacuum_cost_delay; +extern PGDLLIMPORT int vacuum_cost_limit; /* Variables for cost-based parallel vacuum */ extern PGDLLIMPORT pg_atomic_uint32 *VacuumSharedCostBalance; @@ -307,6 +309,8 @@ extern PGDLLIMPORT pg_atomic_uint32 *VacuumActiveNWorkers; extern PGDLLIMPORT int VacuumCostBalanceLocal; extern bool VacuumFailsafeActive; +extern int VacuumCostLimit; +extern double VacuumCostDelay; /* in commands/vacuum.c */ extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel); @@ -346,6 +350,9 @@ extern IndexBulkDeleteResult *vac_cleanup_one_index(IndexVacuumInfo *ivinfo, IndexBulkDeleteResult *istat); extern Size vac_max_items_to_alloc_size(int max_items); +/* In postmaster/autovacuum.c */ +extern void VacuumUpdateCosts(void); + /* in commands/vacuumparallel.c */ extern ParallelVacuumState *parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes, int nrequested_workers, diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 06a86f9ac1..66db1b2c69 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -266,8 +266,6 @@ extern PGDLLIMPORT int max_parallel_maintenance_workers; extern PGDLLIMPORT int VacuumCostPageHit; extern PGDLLIMPORT int VacuumCostPageMiss; extern PGDLLIMPORT int VacuumCostPageDirty; -extern PGDLLIMPORT int VacuumCostLimit; -extern PGDLLIMPORT double VacuumCostDelay; extern PGDLLIMPORT int64 VacuumPageHit; extern PGDLLIMPORT int64 VacuumPageMiss; diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h index c140371b51..65afd1ea1e 100644 --- a/src/include/postmaster/autovacuum.h +++ b/src/include/postmaster/autovacuum.h @@ -63,9 +63,6 @@ extern int StartAutoVacWorker(void); /* called from postmaster when a worker could not be forked */ extern void AutoVacWorkerFailed(void); -/* autovacuum cost-delay balancer */ -extern void AutoVacuumUpdateDelay(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
From 4792ff15f6ab2ce52763dd61a2f374dee948a9cd Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Fri, 31 Mar 2023 10:38:39 -0400 Subject: [PATCH v14 1/4] Make vacuum's failsafe_active a global 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, elevate LVRelState->failsafe_active to a global, VacuumFailsafeActive, which will be checked when determining whether or not to re-enable vacuum cost-related delays. Reviewed-by: Masahiko Sawada <sawada.m...@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com --- src/backend/access/heap/vacuumlazy.c | 16 +++++++--------- src/backend/commands/vacuum.c | 9 +++++++++ src/include/commands/vacuum.h | 1 + 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 639179aa46..2ba85bd3d6 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -153,8 +153,6 @@ typedef struct LVRelState bool aggressive; /* Use visibility map to skip? (disabled by DISABLE_PAGE_SKIPPING) */ bool skipwithvm; - /* Wraparound failsafe has been triggered? */ - bool failsafe_active; /* Consider index vacuuming bypass optimization? */ bool consider_bypass_optimization; @@ -391,7 +389,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, Assert(params->index_cleanup != VACOPTVALUE_UNSPECIFIED); Assert(params->truncate != VACOPTVALUE_UNSPECIFIED && params->truncate != VACOPTVALUE_AUTO); - vacrel->failsafe_active = false; + VacuumFailsafeActive = false; vacrel->consider_bypass_optimization = true; vacrel->do_index_vacuuming = true; vacrel->do_index_cleanup = true; @@ -709,7 +707,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, } else { - if (!vacrel->failsafe_active) + if (!VacuumFailsafeActive) appendStringInfoString(&buf, _("index scan bypassed: ")); else appendStringInfoString(&buf, _("index scan bypassed by failsafe: ")); @@ -2293,7 +2291,7 @@ lazy_vacuum(LVRelState *vacrel) * vacuuming or heap vacuuming. This VACUUM operation won't end up * back here again. */ - Assert(vacrel->failsafe_active); + Assert(VacuumFailsafeActive); } /* @@ -2374,7 +2372,7 @@ lazy_vacuum_all_indexes(LVRelState *vacrel) */ Assert(vacrel->num_index_scans > 0 || vacrel->dead_items->num_items == vacrel->lpdead_items); - Assert(allindexes || vacrel->failsafe_active); + Assert(allindexes || VacuumFailsafeActive); /* * Increase and report the number of index scans. @@ -2616,12 +2614,12 @@ static bool lazy_check_wraparound_failsafe(LVRelState *vacrel) { /* Don't warn more than once per VACUUM */ - if (vacrel->failsafe_active) + if (VacuumFailsafeActive) return true; if (unlikely(vacuum_xid_failsafe_check(&vacrel->cutoffs))) { - vacrel->failsafe_active = true; + VacuumFailsafeActive = true; /* * Abandon use of a buffer access strategy to allow use of all of @@ -2820,7 +2818,7 @@ should_attempt_truncation(LVRelState *vacrel) { BlockNumber possibly_freeable; - if (!vacrel->do_rel_truncate || vacrel->failsafe_active || + if (!vacrel->do_rel_truncate || VacuumFailsafeActive || old_snapshot_threshold >= 0) return false; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index da85330ef4..9724fbce46 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -72,6 +72,15 @@ int vacuum_multixact_freeze_table_age; int vacuum_failsafe_age; int vacuum_multixact_failsafe_age; +/* + * VacuumFailsafeActive is a defined as a global so that we can determine + * whether or not to re-enable cost-based vacuum delay when vacuuming a table. + * If failsafe mode has been engaged, we will not re-enable cost-based delay + * for the table until after vacuuming has completed, regardless of other + * settings. + */ +bool VacuumFailsafeActive = false; + /* * Variables for cost-based parallel vacuum. See comments atop * compute_parallel_delay to understand how it works. diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bdfd96cfec..7b8ee21788 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -306,6 +306,7 @@ extern PGDLLIMPORT pg_atomic_uint32 *VacuumSharedCostBalance; extern PGDLLIMPORT pg_atomic_uint32 *VacuumActiveNWorkers; extern PGDLLIMPORT int VacuumCostBalanceLocal; +extern bool VacuumFailsafeActive; /* in commands/vacuum.c */ extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel); -- 2.37.2