Re: Should vacuum process config file reload more often - Mailing list pgsql-hackers

On Wed, Apr 5, 2023 at 5:05 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Tue, Apr 4, 2023 at 4:27 AM Masahiko Sawada <sawada.mshk@gmail.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