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: <CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
Date: Wed, 5 Apr 2023 21:03:53 +0200
Cc: Melanie Plageman <melanieplageman@gmail.com>,
 Andres Freund <andres@anarazel.de>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
 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: <F2CC9913-5630-48B5-8466-2F33801F24B2@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>
 <CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/F2CC9913-5630-48B5-8466-2F33801F24B2%40yesql.se>
Precedence: bulk

> On 5 Apr 2023, at 20:55, Robert Haas <robertmhaas@gmail.com> wrote:

> Again, I don't think this is something we should try to
> address right now under time pressure, but in the future, I think we
> should consider ripping this behavior out.

I would not be opposed to that, but I wholeheartedly agree that it's not =
the
job of this patch (or any patch at this point in the cycle).

> +               if (autovacuum_vac_cost_limit > 0)
> +                       VacuumCostLimit =3D autovacuum_vac_cost_limit;
> +               else
> +                       VacuumCostLimit =3D vacuum_cost_limit;
> +
> +               /* Only balance limit if no cost-related storage
> parameters specified */
> +               if =
(pg_atomic_unlocked_test_flag(&MyWorkerInfo->wi_dobalance))
> +                       return;
> +               Assert(VacuumCostLimit > 0);
> +
> +               nworkers_for_balance =3D pg_atomic_read_u32(
> +
> &AutoVacuumShmem->av_nworkersForBalance);
> +
> +               /* There is at least 1 autovac worker (this worker). =
*/
> +               if (nworkers_for_balance <=3D 0)
> +                       elog(ERROR, "nworkers_for_balance must be > =
0");
> +
> +               VacuumCostLimit =3D Max(VacuumCostLimit /
> nworkers_for_balance, 1);
>=20
> I think it would be better stylistically to use a temporary variable
> here and only assign the final value to VacuumCostLimit.

I can agree with that.  Another supertiny nitpick on the above is to not =
end a
single-line comment with a period.

> Daniel: Are you intending to commit this?

Yes, my plan is to get it in before feature freeze.  I notice now that I =
had
missed setting myself as committer in the CF to signal this intent, =
sorry about
that.

--
Daniel Gustafsson