public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: 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 17:54:46 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@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>
> On 5 Apr 2023, at 17:29, Melanie Plageman <[email protected]> wrote:
>>
>> I think I wasn't clear in my comment, sorry. I don't have a problem with
>> introducing a new variable to split the balanced value from the GUC value.
>> What I don't think we should do is repurpose an exported symbol into doing a
>> new thing. In the case at hand I think VacuumCostLimit and VacuumCostDelay
>> should remain the backing variables for the GUCs, with vacuum_cost_limit and
>> vacuum_cost_delay carrying the balanced values. So the inverse of what is in
>> the patch now.
>>
>> The risk of these symbols being used in extensions might be very low but on
>> principle it seems unwise to alter a symbol and risk subtle breakage.
>
> I totally see what you are saying. The only complication is that all of
> the other variables used in vacuum code are the camelcase and the gucs
> follow the snake case -- as pointed out in a previous review comment by
> Sawada-san:
Fair point.
>> @@ -83,6 +84,7 @@ int vacuum_cost_limit;
>> */
>> int VacuumCostLimit = 0;
>> double VacuumCostDelay = -1;
>> +static bool vacuum_can_reload_config = false;
>>
>> In vacuum.c, we use snake case for GUC parameters and camel case for
>> other global variables, so it seems better to rename it
>> VacuumCanReloadConfig. Sorry, that's my fault.
>
> This is less of a compelling argument than subtle breakage for extension
> code, though.
How about if we rename the variable into something which also acts at bit as
self documenting why there are two in the first place? Perhaps
BalancedVacuumCostLimit or something similar (I'm terrible with names)?
> I am, however, wondering if extensions expect to have access to the guc
> variable or the global variable -- or both?
Extensions have access to all exported symbols, and I think it's not uncommon
for extension authors to expect to have access to at least read GUC variables.
--
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