public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: Robert Haas <[email protected]>
Cc: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Masahiko Sawada <[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: Wed, 5 Apr 2023 21:03:53 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@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>
<CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
> On 5 Apr 2023, at 20:55, Robert Haas <[email protected]> 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 = autovacuum_vac_cost_limit;
> + else
> + VacuumCostLimit = 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 = pg_atomic_read_u32(
> +
> &AutoVacuumShmem->av_nworkersForBalance);
> +
> + /* There is at least 1 autovac worker (this worker). */
> + if (nworkers_for_balance <= 0)
> + elog(ERROR, "nworkers_for_balance must be > 0");
> +
> + VacuumCostLimit = Max(VacuumCostLimit /
> nworkers_for_balance, 1);
>
> 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
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], [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