Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.2\))
Subject: Re: Should vacuum process config file reload more often
From: Daniel Gustafsson <daniel@yesql.se>
In-Reply-To: <CAD21AoAbsGrFwr9Hn_4ytMUf4u1SBXG7Z2BDCx_ERrBfjEc0eg@mail.gmail.com>
Date: Thu, 6 Apr 2023 14:29:34 +0200
Cc: Melanie Plageman <melanieplageman@gmail.com>,
 Andres Freund <andres@anarazel.de>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 Amit Kapila <amit.kapila16@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <862AF338-C824-4736-8FCA-3971160858E1@yesql.se>
References: <20230329.132155.629765142788133576.horikyota.ntt@gmail.com>
 <20230329.173456.1185961934810139447.horikyota.ntt@gmail.com>
 <CAAKRu_Yg4-kWH_EN7wtpm3z9B+H27UTmOxzzYv1Vg=YdZ6v_rw@mail.gmail.com>
 <CAD21AoCGdxYAJHmE_nXw_MFw1+Qtn3vcThRu5FiadukgQnpWeQ@mail.gmail.com>
 <935191FB-083B-4060-89FC-466F61FAA391@yesql.se>
 <CAAKRu_aoa0YT36q2n3xa9MZYdepeH4Jyf2EVJj=7YdC_1c974Q@mail.gmail.com>
 <CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com>
 <CAD21AoDi4i4tj1Cf9BG0q2iyVhHisQ2xxVBZajEqExX6t6_b3g@mail.gmail.com>
 <CAAKRu_bkg+0WAGTxsy0HP75RzFbPwqz3ctYq1kThjFhSL69upw@mail.gmail.com>
 <1252075.1680547394@sss.pgh.pa.us>
 <20230403190837.qubpnwugfe2k2g46@awork3.anarazel.de>
 <CAAKRu_ap7nhmqtKQ1TRo3LzHG3FK=YpGO=O0019M3BbN7ysm3A@mail.gmail.com>
 <7E06F879-7E20-4A6A-862F-CA72CDC9A323@yesql.se>
 <CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw@mail.gmail.com>
 <CBF0083E-5893-4B35-9498-142A835D03DC@yesql.se>
 <CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@mail.gmail.com>
 <CAD21AoAbsGrFwr9Hn_4ytMUf4u1SBXG7Z2BDCx_ERrBfjEc0eg@mail.gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/862AF338-C824-4736-8FCA-3971160858E1%40yesql.se>
Precedence: bulk

> On 6 Apr 2023, at 08:39, Masahiko Sawada <sawada.mshk@gmail.com> =
wrote:

> Also I agree with
> where to put the log but I think the log message should start with
> lower cases:
>=20
> +                elog(DEBUG2,
> +                         "Autovacuum VacuumUpdateCosts(db=3D%u, =
rel=3D%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.
> + */
>=20
> 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);
>=20
> 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