Re: Should vacuum process config file reload more often

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Melanie Plageman <melanieplageman(at)gmail(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: Should vacuum process config file reload more often
Date: 2023-04-05 05:56:22
Message-ID: CAD21AoBw2rwbf=rrFPVewzkZHA2SXNaEkS+ogTUA7uKmVxyPjg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 5, 2023 at 5:05 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Tue, Apr 4, 2023 at 4:27 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > ---
> > - 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);
> >
> > I think it's better to keep this kind of log in some form for
> > debugging. For example, we can show these values of autovacuum workers
> > in VacuumUpdateCosts().
>
> I added a message to do_autovacuum() after calling VacuumUpdateCosts()
> in the loop vacuuming each table. That means it will happen once per
> table. It's not ideal that I had to move the call to VacuumUpdateCosts()
> behind the shared lock in that loop so that we could access the pid and
> such in the logging message after updating the cost and delay, but it is
> probably okay. Though noone is going to be changing those at this
> point, it still seemed better to access them under the lock.
>
> This does mean we won't log anything when we do change the values of
> VacuumCostDelay and VacuumCostLimit while vacuuming a table. Is it worth
> adding some code to do that in VacuumUpdateCosts() (only when the value
> has changed not on every call to VacuumUpdateCosts())? Or perhaps we
> could add it in the config reload branch that is already in
> vacuum_delay_point()?

Previously, we used to show the pid in the log since a worker/launcher
set other workers' delay costs. But now that the worker sets its delay
costs, we don't need to show the pid in the log. Also, I think it's
useful for debugging and investigating the system if we log it when
changing the values. The log I imagined to add was like:

@@ -1801,6 +1801,13 @@ VacuumUpdateCosts(void)
VacuumCostDelay = vacuum_cost_delay;

AutoVacuumUpdateLimit();
+
+ elog(DEBUG2, "autovacuum update costs (db=%u, rel=%u,
dobalance=%s, cost_limit=%d, cost_delay=%g active=%s failsafe=%s)",
+ MyWorkerInfo->wi_dboid, MyWorkerInfo->wi_tableoid,
+ pg_atomic_unlocked_test_flag(&MyWorkerInfo->wi_dobalance)
? "no" : "yes",
+ VacuumCostLimit, VacuumCostDelay,
+ VacuumCostDelay > 0 ? "yes" : "no",
+ VacuumFailsafeActive ? "yes" : "no");
}
else
{

Regards,

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-04-05 06:59:04 Re: Minimal logical decoding on standbys
Previous Message Greg Stark 2023-04-05 05:41:42 Re: Temporary tables versus wraparound... again