public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Re: Should vacuum process config file reload more often
Date: Thu, 6 Apr 2023 14:29:34 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD21AoAbsGrFwr9Hn_4ytMUf4u1SBXG7Z2BDCx_ERrBfjEc0eg@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CAAKRu_Yg4-kWH_EN7wtpm3z9B+H27UTmOxzzYv1Vg=YdZ6v_rw@mail.gmail.com>
<CAD21AoCGdxYAJHmE_nXw_MFw1+Qtn3vcThRu5FiadukgQnpWeQ@mail.gmail.com>
<[email protected]>
<CAAKRu_aoa0YT36q2n3xa9MZYdepeH4Jyf2EVJj=7YdC_1c974Q@mail.gmail.com>
<CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com>
<CAD21AoDi4i4tj1Cf9BG0q2iyVhHisQ2xxVBZajEqExX6t6_b3g@mail.gmail.com>
<CAAKRu_bkg+0WAGTxsy0HP75RzFbPwqz3ctYq1kThjFhSL69upw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAAKRu_ap7nhmqtKQ1TRo3LzHG3FK=YpGO=O0019M3BbN7ysm3A@mail.gmail.com>
<[email protected]>
<CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw@mail.gmail.com>
<[email protected]>
<CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@mail.gmail.com>
<CAD21AoAbsGrFwr9Hn_4ytMUf4u1SBXG7Z2BDCx_ERrBfjEc0eg@mail.gmail.com>
> On 6 Apr 2023, at 08:39, Masahiko Sawada <[email protected]> wrote:
> Also I agree with
> where to put the log but I think the log message should start with
> lower cases:
>
> + elog(DEBUG2,
> + "Autovacuum VacuumUpdateCosts(db=%u, rel=%u,
In principle I agree, but in this case Autovacuum is a name and should IMO in
userfacing messages start with capital A.
> +/*
> + * autovac_recalculate_workers_for_balance
> + * Recalculate the number of workers to consider, given
> cost-related
> + * storage parameters and the current number of active workers.
> + *
> + * Caller must hold the AutovacuumLock in at least shared mode to access
> + * worker->wi_proc.
> + */
>
> Does it make sense to add Assert(LWLockHeldByMe(AutovacuumLock)) at
> the beginning of this function?
That's probably not a bad idea.
> ---
> /* 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);
>
> Do we really need to have the autovacuum launcher recalculate
> av_nworkersForBalance after reloading the config file? Since the cost
> parameters are not used inautovac_recalculate_workers_for_balance()
> the comment also needs to be updated.
If I understand this comment right; there was a discussion upthread that simply
doing it in both launcher and worker simplifies the code with little overhead.
A comment can reflect that choice though.
--
Daniel Gustafsson
view thread (27+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Should vacuum process config file reload more often
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox