Subject: Should vacuum process config file reload more often

From: Melanie Plageman <[email-masked]>
Date: Thu, 23 Feb 2023 17:08:16 -0500

Hi,

Users may wish to 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). This has been brought up for
autovacuum in [1].

Andres suggested that it might be possible to check ConfigReloadPending
in vacuum_delay_point(), so I thought I would draft a rough patch and
start a discussion.

Since vacuum_delay_point() is also called by analyze and we do not want
to reload the configuration file if we are in a user transaction, I
widened the scope of the in_outer_xact variable in vacuum() and allowed
analyze in a user transaction to default to the current configuration
file reload cadence in PostgresMain().

I don't think I can set and leave vac_in_outer_xact the way I am doing
it in this patch, since I use vac_in_outer_xact in vacuum_delay_point(),
which I believe is reachable from codepaths that would not have called
vacuum(). It seems that if a backend sets it, the outer transaction
commits, and then the backend ends up calling vacuum_delay_point() in a
different way later, it wouldn't be quite right.

Apart from this, one higher level question I have is if there are other
gucs whose modification would make reloading the configuration file
during vacuum/analyze unsafe.

- Melanie

[1] https://www.postgresql.org/message-id/flat/22CA91B4-D341-4075-BD3C-4BAB52AF1E80%40amazon.com#37f05e33d2ce43680f96332fa1c0f3d4
From aea6fbfd93ab12e4e27869b755367ab8454e3eef Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Thu, 23 Feb 2023 15:54:55 -0500
Subject: [PATCH v1] reload config file vac

---
 src/backend/commands/vacuum.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index aa79d9de4d..979d19222d 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 vac_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;
+		vac_in_outer_xact = false;
 	}
 	else
-		in_outer_xact = IsInTransactionBlock(isTopLevel);
+		vac_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 (vac_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(!vac_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, vac_in_outer_xact, vac_strategy);
 
 				if (use_own_xacts)
 				{
@@ -2214,6 +2215,12 @@ vacuum_delay_point(void)
 						 WAIT_EVENT_VACUUM_DELAY);
 		ResetLatch(MyLatch);
 
+		if (ConfigReloadPending && !vac_in_outer_xact)
+		{
+			ConfigReloadPending = false;
+			ProcessConfigFile(PGC_SIGHUP);
+		}
+
 		VacuumCostBalance = 0;
 
 		/* update balance values for workers */
-- 
2.37.2

---

From: Pavel Borisov <[email-masked]>
Date: Fri, 24 Feb 2023 12:42:45 +0400

Hi, Melanie!

> during vacuum/analyze unsafe.

I have a couple of small questions:
Can this patch also read the current GUC value if it's modified by the
SET command, without editing config file?
What will be if we modify config file with mistakes? (When we try to
start the cluster with an erroneous config file it will fail to start,
not sure about re-read config)

Overall the proposal seems legit and useful.

Kind regards,
Pavel Borisov

---

From: Masahiko Sawada <[email-masked]>
Date: Mon, 27 Feb 2023 23:11:53 +0900

Hi,

> in vacuum_delay_point(), so I thought I would draft a rough patch and

In vacuum_delay_point(), we need to update VacuumCostActive too if necessary.

> during vacuum/analyze unsafe.

As far as I know there are not such GUC parameters in the core but
there might be in third-party table AM and index AM extensions. Also,
I'm concerned that allowing to change any GUC parameters during
vacuum/analyze could be a foot-gun in the future. When modifying
vacuum/analyze-related codes, we have to consider the case where any
GUC parameters could be changed during vacuum/analyze. I guess it
would be better to apply the parameter changes for only vacuum delay
related parameters. For example, autovacuum launcher advertises the
values of the vacuum delay parameters on the shared memory not only
for autovacuum processes but also for manual vacuum/analyze processes.
Both processes can update them accordingly in vacuum_delay_point().

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

---

From: Melanie Plageman <[email-masked]>
Date: Wed, 1 Mar 2023 14:54:05 -0500

Thanks for the feedback and questions, Pavel!

> SET command, without editing config file?

If a user sets a guc like vacuum_cost_limit with SET, this only modifies
the value for that session. That wouldn't affect the in-progress vacuum
you initiated from that session because you would have to wait for the
vacuum to complete before issuing the SET command.

> not sure about re-read config)

If you manually add an invalid valid to your postgresql.conf, when it is
reloaded, the existing value will remain unchanged and an error will be
logged. If you attempt to change the guc value to an invalid value with
ALTER SYSTEM, the ALTER SYSTEM command will fail and the existing value
will remain unchanged.

- Melanie

---

From: Melanie Plageman <[email-masked]>
Date: Wed, 1 Mar 2023 20:41:14 -0500

> In vacuum_delay_point(), we need to update VacuumCostActive too if necessary.

Yes, good point. Thank you!

> different way later, it wouldn't be quite right.

Perhaps I could just set in_outer_xact to false in the PG_FINALLY()
section in vacuum() to avoid this problem.

On Wed, Mar 1, 2023 at 7:15 PM Andres Freund <[email-masked]> wrote:
> On 2023-02-28 11:16:45 +0900, Masahiko Sawada wrote:
> > On Tue, Feb 28, 2023 at 10:21 AM Andres Freund <[email-masked]> wrote:
> > > On 2023-02-27 23:11:53 +0900, Masahiko Sawada wrote:
> > > > Also, I'm concerned that allowing to change any GUC parameters during
> > > > vacuum/analyze-related codes, we have to consider the case where any GUC
> > > > parameters could be changed during vacuum/analyze.
> > >
> > > What kind of scenario are you thinking of?
> >
> > For example, I guess we will need to take care of changes of
> > maintenance_work_mem. Currently we initialize the dead tuple space at
> > the beginning of lazy vacuum, but perhaps we would need to
> > enlarge/shrink it based on the new value?
>
> I don't think we need to do anything about that initially, just because the
> config can be changed in a more granular way, doesn't mean we have to react to
> every change for the current operation.

Perhaps we can mention in the docs that a change to maintenance_work_mem
will not take effect in the middle of vacuuming a table. But, Ithink it probably
isn't needed.

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.

I started writing a little helper that could be used to update these
workerinfo->wi_cost_delay/limit in vacuum_delay_point(), but I notice
when they are first set, we consider the autovacuum table options. So,
I suppose I would need to consider these when updating
wi_cost_delay/limit later as well? (during vacuum_delay_point() or
in AutoVacuumUpdateDelay())

I wasn't quite sure because I found these chained ternaries rather
difficult to interpret, but I think table_recheck_autovac() is saying
that the autovacuum table options override all other values for
vac_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;

i.e. this?

  if (avopts && avopts->vacuum_cost_delay >= 0)
    vac_cost_delay = avopts->vacuum_cost_delay;
  else if (autovacuum_vac_cost_delay >= 0)
    vac_cost_delay = autovacuum_vacuum_cost_delay;
  else
    vac_cost_delay = VacuumCostDelay

- Melanie

---

From: Masahiko Sawada <[email-masked]>
Date: Thu, 2 Mar 2023 16:36:09 +0900

> will not take effect in the middle of vacuuming a table. But, Ithink it probably

Agreed.

> 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.

> 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.

>  but I notice

Yes, if the table has autovacuum table options, we use these values
and the table is excluded from the balancing algorithm I mentioned
above. See the code from table_recheck_autovac(),

       /*
        * If any of the cost delay parameters has been set individually for
        * this table, disable the balancing algorithm.
        */
       tab->at_dobalance =
           !(avopts && (avopts->vacuum_cost_limit > 0 ||
                        avopts->vacuum_cost_delay > 0));

So if the table has autovacuum table options, the vacuum delay
parameters probably should be updated by ALTER TABLE, not by reloading
the config file.

Regards,

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 2 Mar 2023 18:37:43 -0500

> workers. I'm not sure why we don't update workers' wi_cost_delay,

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.

> that the autovacuum launcher sets workers' delay parameters and they

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.

> parameters probably should be updated by ALTER TABLE, not by reloading

Yes, if the table has autovacuum table options, I think the user is
out-of-luck until the relation is done being vacuumed because the ALTER
TABLE will need to get a lock.

- Melanie

---

From: Melanie Plageman <[email-masked]>
Date: Sun, 5 Mar 2023 15:26:12 -0500

> differs between autovacuum workers and explicit vacuum is quite tangled

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 <[email-masked]>
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(-)

index aa79d9de4d..f6cea30168 100644
+static bool analyze_in_outer_xact = false;

 /*
 
 	const char *stmttype;
 
 	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);
 
 	/*
+		else if (analyze_in_outer_xact)
+		Assert(!analyze_in_outer_xact);

 				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

---

From: Masahiko Sawada <[email-masked]>
Date: Tue, 7 Mar 2023 14:09:54 +0900

> from the previous versions.

Thank you for updating the patch!

> certain things. My code needs a better solution.

While it's true that wi_cost_delay doesn't need to be shared, it seems
to make the logic somewhat complex. We need to handle cost_delay in a
different way from other vacuum-related parameters and we need to make
sure av[_use]_table_option_cost_delay are set properly. Removing
wi_cost_delay from WorkerInfoData saves 8 bytes shared memory per
autovacuum worker but it might be worth considering to keep
wi_cost_delay for simplicity.

---
 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;

Is there any case where the autovacuum launcher calls
AutoVacuumUpdateDelay() function?

---
In at autovac_balance_cost(), we have,

    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);
    :
    /* not set? nothing to do */
    if (vac_cost_limit <= 0 || vac_cost_delay <= 0)
        return;

IIUC if autovacuum_vac_cost_delay is changed to 0 during autovacuums
running, their vacuum delay parameters are not changed. It's not a bug
of the patch but I think we can fix it in this patch.

> wi_cost_delay from shared memory without holding a lock.

Indeed.

> can read from it less frequently?

Yeah, acquiring the lwlock for every call to vacuum_delay_point()
seems to be harmful. One idea would be to have one sig_atomic_t
variable in WorkerInfoData and autovac_balance_cost() set it to true
after rebalancing the worker's cost-limit. The worker can check it
without locking and update its delay parameters if the flag is true.

> Also not sure how the patch interacts with failsafe autovac and parallel

Good point.

When entering the failsafe mode, we disable the vacuum delays (see
lazy_check_wraparound_failsafe()). We need to keep disabling the
vacuum delays even after reloading the config file. One idea is to
have another global variable indicating we're in the failsafe mode.
vacuum_delay_point() doesn't update VacuumCostActive if the flag is
true.

As far as I can see we don't need special treatments for parallel
vacuum cases since it works only in manual vacuum. It calculates the
sleep time based on the shared cost balance and how much the worker
did I/O but the basic mechanism is the same as non-parallel case.

Regards,

---

From: Jim Nasby <[email-masked]>
Date: Wed, 8 Mar 2023 11:42:31 -0600

>>>> enlarge/shrink it based on the new value?
Doesn't the dead tuple space grow as needed? Last I looked we don't 
allocate up to 1GB right off the bat.
>> will not take effect in the middle of vacuuming a table. But, Ithink it probably

I disagree that there's no need for this. Sure, if 
maintenance_work_memory is 10MB then it's no big deal to just abandon 
your current vacuum and start a new one, but the index vacuuming phase 
with maintenance_work_mem set to say 500MB can take quite a while. 
Forcing a user to either suck it up or throw everything in the phase 
away isn't terribly good.

Of course, if the patch that eliminates the 1GB vacuum limit gets 
committed the situation will be even worse.

While it'd be nice to also honor maintenance_work_mem getting set lower, 
I don't see any need to go through heroics to accomplish that. Simply 
recording the change and honoring it for future attempts to grow the 
memory and on future passes through the heap would be plenty.

All that said, don't let these suggestions get in the way of committing 
this. Just having the ability to tweak cost parameters would be a win.

---

From: Andres Freund <[email-masked]>
Date: Wed, 8 Mar 2023 16:27:29 -0800

Hi,

On 2023-03-08 11:42:31 -0600, Jim Nasby wrote:
> Doesn't the dead tuple space grow as needed? Last I looked we don't allocate
> up to 1GB right off the bat.
> I disagree that there's no need for this. Sure, if maintenance_work_memory
> is 10MB then it's no big deal to just abandon your current vacuum and start
> a new one, but the index vacuuming phase with maintenance_work_mem set to
> say 500MB can take quite a while. Forcing a user to either suck it up or
> throw everything in the phase away isn't terribly good.
> 
> Of course, if the patch that eliminates the 1GB vacuum limit gets committed
> the situation will be even worse.
> 
> While it'd be nice to also honor maintenance_work_mem getting set lower, I
> don't see any need to go through heroics to accomplish that. Simply
> recording the change and honoring it for future attempts to grow the memory
> and on future passes through the heap would be plenty.
> 

Nobody said anything about it not being useful to react to m_w_m changes, just
that it's not required to make some progress . So I really don't understand
what the point of your comment is.

Greetings,

Andres Freund

---

From: John Naylor <[email-masked]>
Date: Thu, 9 Mar 2023 14:47:19 +0700

Incorrect.

If you're referring to the proposed tid store, I'd be interested in seeing
a reproducible test case with a m_w_m over 1GB where it makes things worse
than the current state of affairs.

--
John Naylor
EDB: http://www.enterprisedb.com

---

From: Masahiko Sawada <[email-masked]>
Date: Thu, 9 Mar 2023 22:19:11 +0900

On Thu, Mar 9, 2023 at 4:47 PM John Naylor <[email-masked]> wrote:
> > Doesn't the dead tuple space grow as needed? Last I looked we don't allocate up to 1GB right off the bat.
>
> Incorrect.
>
> > Of course, if the patch that eliminates the 1GB vacuum limit gets committed the situation will be even worse.
>
> If you're referring to the proposed tid store, I'd be interested in seeing a reproducible test case with a m_w_m over 1GB where it makes things worse than the current state of affairs.

And I think that the tidstore makes it easy to react to
maintenance_work_mem changes. We don't need to enlarge it and just
update its memory limit at an appropriate time.

Regards,

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 9 Mar 2023 21:22:53 -0500

> wi_cost_delay for simplicity.

Ah, it turns out we can't really remove wi_cost_delay from WorkerInfo
anyway because the launcher doesn't know anything about table options
and so the workers have to keep an updated wi_cost_delay that the
launcher or other autovac workers who are not vacuuming that table can
read from when calculating the new limit in autovac_balance_cost().

However, wi_cost_delay is a double, so if we start updating it on config
reload in vacuum_delay_point(), we definitely need some protection
against torn reads.

The table options can only change when workers start vacuuming a new
table, so maybe there is some way to use this to solve this problem?

> without locking and update its delay parameters if the flag is true.

Maybe we can do something like this with the table options values?

- Melanie

---

From: Masahiko Sawada <[email-masked]>
Date: Fri, 10 Mar 2023 12:26:23 +0900

> read from when calculating the new limit in autovac_balance_cost().

IIUC if any of the cost delay parameters has been set individually,
the autovacuum worker is excluded from the balance algorithm.

> Maybe we can do something like this with the table options values?

Since an autovacuum that uses any of table option cost delay
parameters is excluded from the balancing algorithm, the launcher
doesn't need to notify such workers of changes of the cost-limit, no?

Regards,

---

From: Melanie Plageman <[email-masked]>
Date: Fri, 10 Mar 2023 18:11:23 -0500

Quotes below are combined from two of Sawada-san's emails.

I've also attached a patch with my suggested current version.

> the autovacuum worker is excluded from the balance algorithm.

Ah, yes! That's right. So it is not a problem. Then I still think
removing wi_cost_delay from the worker info makes sense. wi_cost_delay
is a double and can't easily be accessed atomically the way
wi_cost_limit can be.

Keeping the cost delay local to the backends also makes it clear that
cost delay is not something that should be written to by other backends
or that can differ from worker to worker. Without table options in the
picture, the cost delay should be the same for any worker who has
reloaded the config file.

As for the cost limit safe access issue, maybe we can avoid a LWLock
acquisition for reading wi_cost_limit by using an atomic similar to what
you suggested here for "did_rebalance".

> without locking and update its delay parameters if the flag is true.

Instead of having the atomic indicate whether or not someone (launcher
or another worker) did a rebalance, it would simply store the current
cost limit. Then the worker can normally access it with a simple read.

My rationale is that if we used an atomic to indicate whether or not we
did a rebalance ("did_rebalance"), we would have the same cache
coherency guarantees as if we just used the atomic for the cost limit.
If we read from the "did_rebalance" variable and missed someone having
written to it on another core, we still wouldn't get around to checking
the wi_cost_limit variable in shared memory, so it doesn't matter that
we bothered to keep it in shared memory and use a lock to access it.

I noticed we don't allow wi_cost_limit to ever be less than 0, so we
could store wi_cost_limit in an atomic uint32.

I'm not sure if it is okay to do pg_atomic_read_u32() and
pg_atomic_unlocked_write_u32() or if we need pg_atomic_write_u32() in
most cases.

I've implemented the atomic cost limit in the attached patch. Though,
I'm pretty unsure about how I initialized the atomics in
AutoVacuumShmemInit()...

If the consensus is that it is simply too confusing to take
wi_cost_delay out of WorkerInfo, we might be able to afford using a
shared lock to access it because we won't call AutoVacuumUpdateDelay()
on every invocation of vacuum_delay_point() -- only when we've reloaded
the config file.

One potential option to avoid taking a shared lock on every call to
AutoVacuumUpdateDelay() is to set a global variable to indicate that we
did update it (since we are the only ones updating it) and then only
take the shared LWLock in AutoVacuumUpdateDelay() if that flag is true.

> AutoVacuumUpdateDelay() function?

I had meant to add it to HandleAutoVacLauncherInterrupts() after
reloading the config file (done in attached patch). When using the
global variables for cost delay (instead of wi_cost_delay in worker
info), the autovac launcher also has to do the check in the else branch
of AutoVacuumUpdateDelay()

        VacuumCostDelay = autovacuum_vac_cost_delay >= 0 ?
            autovacuum_vac_cost_delay : VacuumCostDelay;

to make sure VacuumCostDelay is correct for when it calls
autovac_balance_cost().

This also made me think about whether or not we still need cost_limit_base.
It is used to ensure that autovac_balance_cost() never ends up setting
workers' wi_cost_limits above the current autovacuum_vacuum_cost_limit
(or VacuumCostLimit). However, the launcher and all the workers should
know what the value is without cost_limit_base, no?

> of the patch but I think we can fix it in this patch.

Yes, currently (in master) wi_cost_delay does not get updated anywhere.
In my patch, the global variable we are using for delay is updated but
it is not done in autovac_balance_cost().

> vacuum_delay_point() doesn't update VacuumCostActive if the flag is

I think we might not need to do this. Other than in
lazy_check_wraparound_failsafe(), VacuumCostActive is only updated in
two places:

1) in vacuum() which autovacuum will call per table. And failsafe is
reset per table as well.

2) in vacuum_delay_point(), but, since VacuumCostActive will already be
false when we enter vacuum_delay_point() the next time after
lazy_check_wraparound_failsafe(), we won't set VacuumCostActive there.

Thanks again for the detailed feedback!

- Melanie
From 97b6ebc3eaa3d8ec53c25822c6cad2d2bbe22837 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Sun, 5 Mar 2023 14:39:16 -0500
Subject: [PATCH v3] Reload config file more often while vacuuming

---
 src/backend/commands/vacuum.c       |  38 +++++++---
 src/backend/postmaster/autovacuum.c | 113 ++++++++++++++++++++--------
 src/include/postmaster/autovacuum.h |   2 +
 3 files changed, 114 insertions(+), 39 deletions(-)

index 2e12baf8eb..9d5ce846a5 100644

 /*
@@ -313,8 +315,7 @@ vacuum(List *relations, VacuumParams *params,
 	static bool in_vacuum = false;
 
 	const char *stmttype;
 
 	Assert(params != NULL);
 
@@ -331,10 +332,10 @@ vacuum(List *relations, VacuumParams *params,
 
 	/*
@@ -456,7 +457,7 @@ vacuum(List *relations, VacuumParams *params,
@@ -474,7 +475,7 @@ vacuum(List *relations, VacuumParams *params,
 
 		/* ActiveSnapshot is not set by autovacuum */
 		if (ActiveSnapshotSet())
@@ -526,7 +527,7 @@ vacuum(List *relations, VacuumParams *params,
 				}

 				if (use_own_xacts)
 				{
@@ -549,6 +550,7 @@ vacuum(List *relations, VacuumParams *params,
 	{
 		in_vacuum = false;
 		VacuumCostActive = false;
+		analyze_in_outer_xact = false;
 	}
 	PG_END_TRY();
 
@@ -2238,10 +2240,28 @@ vacuum_delay_point(void)
 						 WAIT_EVENT_VACUUM_DELAY);
 		ResetLatch(MyLatch);
 
+		/*

index c0e2e00a7e..a9b7217638 100644

@@ -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,8 +227,7 @@ typedef struct WorkerInfoData
 	TimestampTz wi_launchtime;
 	bool		wi_dobalance;
 	bool		wi_sharedrel;
-	double		wi_cost_delay;
-	int			wi_cost_limit;
+	pg_atomic_uint32 wi_cost_limit;
 	int			wi_cost_limit_base;
 } WorkerInfoData;
 
@@ -743,6 +744,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 					worker = AutoVacuumShmem->av_startingWorker;
 					worker->wi_dboid = InvalidOid;
 					worker->wi_tableoid = InvalidOid;
+					pg_atomic_write_u32(&MyWorkerInfo->wi_cost_limit, 0);
 					worker->wi_sharedrel = false;
 					worker->wi_proc = NULL;
 					worker->wi_launchtime = 0;
@@ -815,6 +817,7 @@ HandleAutoVacLauncherInterrupts(void)
 		ConfigReloadPending = false;
 		ProcessConfigFile(PGC_SIGHUP);
 
+		AutoVacuumUpdateDelay();
 		/* shutdown requested in config file? */
 		if (!AutoVacuumingActive())
 			AutoVacLauncherShutdown();
@@ -1756,8 +1759,7 @@ 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;
+		pg_atomic_write_u32(&MyWorkerInfo->wi_cost_limit, 0);
 		MyWorkerInfo->wi_cost_limit_base = 0;
 		dlist_push_head(&AutoVacuumShmem->av_freeWorkers,
 						&MyWorkerInfo->wi_links);
@@ -1780,13 +1782,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;
+
+	VacuumCostLimit = pg_atomic_read_u32(&MyWorkerInfo->wi_cost_limit);
+}
+
 /*
  * autovac_balance_cost
  *		Recalculate the cost limit setting for each active worker.
@@ -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)
 		{
 			int			limit = (int)
 			(cost_avail * worker->wi_cost_limit_base / cost_total);
@@ -1855,17 +1881,15 @@ autovac_balance_cost(void)
 			 * 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);
+			pg_atomic_unlocked_write_u32(&worker->wi_cost_limit,
+										 Max(Min(limit, worker->wi_cost_limit_base), 1));
 		}
 
+				 pg_atomic_read_u32(&worker->wi_cost_limit), worker->wi_cost_limit_base);
 	}
 }
 
@@ -2326,6 +2350,13 @@ do_autovacuum(void)
 			ConfigReloadPending = false;
 			ProcessConfigFile(PGC_SIGHUP);
 
+			/*
@@ -2424,21 +2455,22 @@ do_autovacuum(void)

 		/* 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;
+		/* this cannot exceed 10000 anyway */
+		pg_atomic_unlocked_write_u32(&MyWorkerInfo->wi_cost_limit,
+									 tab->at_vacuum_cost_limit);
 		MyWorkerInfo->wi_cost_limit_base = tab->at_vacuum_cost_limit;
+		AutoVacuumUpdateLimit();
 
 		/* do a balance */
 		autovac_balance_cost();

@@ -2569,6 +2601,8 @@ deleted:
 		{
 
@@ -2771,7 +2805,11 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 
 	/*
@@ -2802,7 +2840,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 +2849,16 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		 * defaults, autovacuum's own first and plain vacuum second.
 		 */

@@ -2882,7 +2923,6 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
@@ -2895,6 +2935,11 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 
 	heap_freetuple(classTup);
 	return tab;
@@ -3361,6 +3406,7 @@ AutoVacuumShmemInit(void)
 	{
 		WorkerInfo	worker;
 		int			i;
+		dlist_iter	iter;
 
 		Assert(!found);
 
@@ -3374,10 +3420,17 @@ 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);
+
+		dlist_foreach(iter, &AutoVacuumShmem->av_freeWorkers)
+			pg_atomic_init_u32(
+							   &(dlist_container(WorkerInfoData, wi_links, iter.cur))->wi_cost_limit,
+							   0);
+
 	}
 	else
 		Assert(found);
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);

---

From: Melanie Plageman <[email-masked]>
Date: Fri, 10 Mar 2023 19:34:44 -0500

> on every invocation of vacuum_delay_point() -- only when we've reloaded

One such implementation is attached.

- Melanie
From f0029f1e410852b41b5562939051ede86235508b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Fri, 10 Mar 2023 18:33:47 -0500
Subject: [PATCH v4] vacuum reloads config file more often

---
 src/backend/commands/vacuum.c       | 38 ++++++++++++----
 src/backend/postmaster/autovacuum.c | 69 +++++++++++++++++++++++------
 src/include/postmaster/autovacuum.h |  2 +
 3 files changed, 87 insertions(+), 22 deletions(-)

 /*
 
 	const char *stmttype;
 
 	Assert(params != NULL);

 	/*

 
 				if (use_own_xacts)
 				{

+		/*

 		/* 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..7a202b2bd9 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -226,7 +226,7 @@ typedef struct WorkerInfoData
 	bool		wi_dobalance;
 	bool		wi_sharedrel;
 	double		wi_cost_delay;
-	int			wi_cost_limit;
+	pg_atomic_uint32 wi_cost_limit;
 	int			wi_cost_limit_base;
 } WorkerInfoData;
 
@@ -743,6 +743,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 					worker = AutoVacuumShmem->av_startingWorker;
 					worker->wi_dboid = InvalidOid;
 					worker->wi_tableoid = InvalidOid;
+					pg_atomic_write_u32(&MyWorkerInfo->wi_cost_limit, 0);
 					worker->wi_sharedrel = false;
 					worker->wi_proc = NULL;
 					worker->wi_launchtime = 0;
@@ -1757,7 +1758,7 @@ FreeWorkerInfo(int code, Datum arg)
 		MyWorkerInfo->wi_launchtime = 0;
 		MyWorkerInfo->wi_dobalance = false;
 		MyWorkerInfo->wi_cost_delay = 0;
-		MyWorkerInfo->wi_cost_limit = 0;
+		pg_atomic_write_u32(&MyWorkerInfo->wi_cost_limit, 0);
 		MyWorkerInfo->wi_cost_limit_base = 0;
 		dlist_push_head(&AutoVacuumShmem->av_freeWorkers,
 						&MyWorkerInfo->wi_links);
@@ -1780,11 +1781,36 @@ FreeWorkerInfo(int code, Datum arg)
 void
 AutoVacuumUpdateDelay(void)
 {
-	if (MyWorkerInfo)
+	if (!MyWorkerInfo)
+		return;
+
+	LWLockAcquire(AutovacuumLock, LW_SHARED);
+
+	if (MyWorkerInfo->wi_dobalance)
 	{
-		VacuumCostDelay = MyWorkerInfo->wi_cost_delay;
-		VacuumCostLimit = MyWorkerInfo->wi_cost_limit;
+		VacuumCostDelay = autovacuum_vac_cost_delay >= 0 ?
+			autovacuum_vac_cost_delay : VacuumCostDelay;
+
+		MyWorkerInfo->wi_cost_delay = VacuumCostDelay;
 	}
+	else
+		VacuumCostDelay = MyWorkerInfo->wi_cost_delay;
+
+	LWLockRelease(AutovacuumLock);
+
+}
+
+/*
+ * Helper for vacuum_delay_point() to allow workers to read their
+ * wi_cost_limit.
+ */
+void
+AutoVacuumUpdateLimit(void)
+{
+	if (!MyWorkerInfo)
+		return;
+
+	VacuumCostLimit = pg_atomic_read_u32(&MyWorkerInfo->wi_cost_limit);
 }
 
 /*
@@ -1855,16 +1881,15 @@ 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)",
 				 worker->wi_proc->pid, worker->wi_dboid, worker->wi_tableoid,
 				 worker->wi_dobalance ? "yes" : "no",
-				 worker->wi_cost_limit, worker->wi_cost_limit_base,
+				 pg_atomic_read_u32(&worker->wi_cost_limit), worker->wi_cost_limit_base,
 				 worker->wi_cost_delay);
 	}
 }
@@ -2326,6 +2351,13 @@ do_autovacuum(void)
 			ConfigReloadPending = false;
 			ProcessConfigFile(PGC_SIGHUP);
 
+			/*
@@ -2424,21 +2456,22 @@ do_autovacuum(void)
 
+
 
 		MyWorkerInfo->wi_cost_delay = tab->at_vacuum_cost_delay;
 
 		/* do a balance */
 		autovac_balance_cost();

@@ -2569,6 +2602,8 @@ deleted:
 		{
 
 		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -3361,6 +3396,7 @@ AutoVacuumShmemInit(void)
 	{
 		WorkerInfo	worker;
 		int			i;
+		dlist_iter	iter;
 
 		Assert(!found);
 
@@ -3374,10 +3410,17 @@ AutoVacuumShmemInit(void)
 		worker = (WorkerInfo) ((char *) AutoVacuumShmem +
 							   MAXALIGN(sizeof(AutoVacuumShmemStruct)));
 
+