public inbox for [email protected]
help / color / mirror / Atom feedFrom: Melanie Plageman <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Re: Should vacuum process config file reload more often
Date: Fri, 24 Mar 2023 13:27:45 -0400
Message-ID: <CAAKRu_a39kpGO_SxWEGAHMEHFAOACk3wwQJhRBaN+7Ke9ua_qg@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoCn3-tAB1HgNXY-_B9yaRES0u2gyifw=ABchNV8FkswZg@mail.gmail.com>
References: <CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7+DXhrjDA0gw@mail.gmail.com>
<CAD21AoC=b09pzWNyPYuti0M2DQwy57CGF9UWWb_+v=gdWeNG=A@mail.gmail.com>
<CAAKRu_bR2VrTUskR24GkXV15SxPaCiHYD=soOuWZcT-rGDcA8Q@mail.gmail.com>
<CAD21AoB=EC6wzjjqDXKmNEhu_BhGg0g_Gc50yh_zvXqiDC_OSg@mail.gmail.com>
<CAAKRu_buP5wzsho3qNw5o9_R0pF69FRM5hgCmr-mvXmGXwdA7A@mail.gmail.com>
<CAAKRu_YWNKs2=mE27C1W8VxPJa1UR+YVuckw5kYzWowTStCHiQ@mail.gmail.com>
<CAD21AoAB-jtKvYd-QbnhbanoY+-PDCXvrX-JhBUqGFrB6e7eTw@mail.gmail.com>
<CAAKRu_Zir77=EAQxytN4Bc-P-==1YLC9KiK=kqPricjVzac8ng@mail.gmail.com>
<CAD21AoCBqrzCqXPGjhVHJwJThYW_CmBOpWL2J_6Bb_4qMwcsuw@mail.gmail.com>
<CAAKRu_YxDh-31zzF9vA=6FfggE8-mQo8bwxCxaFqY=pb8kTTag@mail.gmail.com>
<CAD21AoC=BRMwUpVDf1tzDa13--dLhuq=c2XQ_KAYA4Qx3cS=kA@mail.gmail.com>
<CAAKRu_Z4NdHk6EofruFzrasO10bzQYCNuvZ=Rri1wJ6vwoV4-g@mail.gmail.com>
<CAD21AoAJ6medGOeLo4CLkySSPba7yt9UphD-1Sxx+4LY-ixoqw@mail.gmail.com>
<[email protected]>
<CAAKRu_YraQ1mtqRJSqv+rvp0Z3VSWhy19gEkAoYqRkmOFRbYXw@mail.gmail.com>
<CAD21AoCn3-tAB1HgNXY-_B9yaRES0u2gyifw=ABchNV8FkswZg@mail.gmail.com>
On Fri, Mar 24, 2023 at 1:21 AM Masahiko Sawada <[email protected]> wrote:
>
> On Fri, Mar 24, 2023 at 9:27 AM Melanie Plageman
> <[email protected]> wrote:
> >
> > On Thu, Mar 23, 2023 at 2:09 AM Masahiko Sawada <[email protected]> wrote:
> > > On Sun, Mar 19, 2023 at 7:47 AM Melanie Plageman <[email protected]> wrote:
> > > Do we need to calculate the number of workers running with
> > > nworkers_for_balance by iterating over the running worker list? I
> > > guess autovacuum workers can increment/decrement it at the beginning
> > > and end of vacuum.
> >
> > I don't think we can do that because if a worker crashes, we have no way
> > of knowing if it had incremented or decremented the number, so we can't
> > adjust for it.
>
> What kind of crash are you concerned about? If a worker raises an
> ERROR, we can catch it in PG_CATCH() block. If it's a FATAL, we can do
> that in FreeWorkerInfo(). A PANIC error ends up crashing the entire
> server.
Yes, but what about a worker that segfaults? Since table AMs can define
relation_vacuum(), this seems like a real possibility.
I'll address your other code feedback in the next version.
I realized nworkers_for_balance should be initialized to 0 and not 1 --
1 is misleading since there are often 0 autovac workers. We just never
want to use nworkers_for_balance when it is 0. But, workers put a floor
of 1 on the number when they divide limit/nworkers_for_balance (since
they know there must be at least one worker right now since they are a
worker). I thought about whether or not they should call
autovac_balance_cost() if they find that nworkers_for_balance is 0 when
updating their own limit, but I'm not sure.
> > > I need another few readthroughs to figure out of VacuumFailsafeActive does what
> > > I think it does, and should be doing, but in general I think this is a good
> > > idea and a patch in good condition close to being committable.
>
> Another approach would be to make VacuumCostActive a ternary value:
> on, off, and never. When we trigger the failsafe mode we switch it to
> never, meaning that it never becomes active even after reloading the
> config file. A good point is that we don't need to add a new global
> variable, but I'm not sure it's better than the current approach.
Hmm, this is interesting. I don't love the word "never" since it kind of
implies a duration longer than the current table being vacuumed. But we
could find a different word or just document it well. For clarity, we
might want to call it failsafe_mode or something.
I wonder if the primary drawback to converting
LVRelState->failsafe_active to a global VacuumFailsafeActive is just the
general rule of limiting scope to the minimum needed.
- Melanie
view thread (11+ 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]
Subject: Re: Should vacuum process config file reload more often
In-Reply-To: <CAAKRu_a39kpGO_SxWEGAHMEHFAOACk3wwQJhRBaN+7Ke9ua_qg@mail.gmail.com>
* 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