From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, amit(dot)kapila16(at)gmail(dot)com |
Subject: | Re: Should vacuum process config file reload more often |
Date: | 2023-03-31 19:09:21 |
Message-ID: | CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 31, 2023 at 10:31 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Thu, Mar 30, 2023 at 3:26 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >
> > > On 30 Mar 2023, at 04:57, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > > As another idea, why don't we use macros for that? For example,
> > > suppose VacuumCostStatus is like:
> > >
> > > typedef enum VacuumCostStatus
> > > {
> > > VACUUM_COST_INACTIVE_LOCKED = 0,
> > > VACUUM_COST_INACTIVE,
> > > VACUUM_COST_ACTIVE,
> > > } VacuumCostStatus;
> > > VacuumCostStatus VacuumCost;
> > >
> > > non-vacuum code can use the following macros:
> > >
> > > #define VacuumCostActive() (VacuumCost == VACUUM_COST_ACTIVE)
> > > #define VacuumCostInactive() (VacuumCost <= VACUUM_COST_INACTIVE) //
> > > or we can use !VacuumCostActive() instead.
> >
> > I'm in favor of something along these lines. A variable with a name that
> > implies a boolean value (active/inactive) but actually contains a tri-value is
> > easily misunderstood. A VacuumCostState tri-value variable (or a better name)
> > with a set of convenient macros for extracting the boolean active/inactive that
> > most of the code needs to be concerned with would more for more readable code I
> > think.
>
> The macros are very error-prone. I was just implementing this idea and
> mistakenly tried to set the macro instead of the variable in multiple
> places. Avoiding this involves another set of macros, and, in the end, I
> think the complexity is much worse. Given the reviewers' uniform dislike
> of VacuumCostInactive, I favor going back to two variables
> (VacuumCostActive + VacuumFailsafeActive) and moving
> LVRelState->failsafe_active to the global VacuumFailsafeActive.
>
> I will reimplement this in the next version.
>
> On the subject of globals, the next version will implement
> Horiguchi-san's proposal to separate GUC variables from the globals used
> in the code (quoted below). It should hopefully reduce the complexity of
> this patchset.
>
> > Although it's somewhat unrelated to the goal of this patch, I think we
> > should clean up the code tidy before proceeding. Shouldn't we separate
> > the actual parameters from the GUC base variables, and sort out the
> > all related variaghble? (something like the attached, on top of your
> > patch.)
Attached is v12. It has a number of updates, including a commit to
separate VacuumCostLimit and VacuumCostDelay from the gucs
vacuum_cost_limit and vacuum_cost_delay, and a return to
VacuumCostActive.
- Melanie
Attachment | Content-Type | Size |
---|---|---|
v12-0001-Make-vacuum-s-failsafe_active-a-global.patch | text/x-patch | 4.9 KB |
v12-0003-VACUUM-reloads-config-file-more-often.patch | text/x-patch | 6.2 KB |
v12-0004-Autovacuum-refreshes-cost-based-delay-params-mor.patch | text/x-patch | 17.7 KB |
v12-0002-Separate-vacuum-cost-variables-from-gucs.patch | text/x-patch | 10.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2023-03-31 19:17:06 | Re: running logical replication as the subscription owner |
Previous Message | Imseih (AWS), Sami | 2023-03-31 18:06:46 | Re: [BUG] pg_stat_statements and extended query protocol |