public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Thom Brown <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Thu, 5 Dec 2024 20:28:10 +0900
Message-ID: <CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
References: <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
<CA+HiwqHzKO9FT-CjFWo6OmkiCSYmbPspKXVex96tOBKf6S_x_w@mail.gmail.com>
<CA+TgmoZGWyMXutfen-NNv9=QM7eCHn9R1bpLZ9N4sRURMOCK2A@mail.gmail.com>
<CA+HiwqHNb9jrwOFHfmASfiGc=SnqXs7THwQ_Rta=z+ognYV8qw@mail.gmail.com>
<CA+HiwqH9u1RWn9OEa=VQQpJagB0hDLCY+=fSyBC4ZkeU6Gg2HA@mail.gmail.com>
<CA+HiwqFMWt2MQVqhp2rZA8=ugPVD=5uW10QCdK_NpoyWyFLe-g@mail.gmail.com>
<CA+HiwqGBpw_JNwkwZjQ2YaqTWrDjn9L5jpuc+nS8=a55SPD+UA@mail.gmail.com>
<CA+HiwqFGz2uShfU=qtack9gii6Kzyjv1V66tJJBYBN8Acb4uTA@mail.gmail.com>
<CA+HiwqE7+iwMH4NYtFi28Pt9fT_gRW+Gt-=CvOX=Pkquo=AN8w@mail.gmail.com>
<CA+TgmobO_6irkJGkzkxHTR=kza_CG+0idAhFUWqGfXCVQQmuPg@mail.gmail.com>
<CA+HiwqH45ZCQkWoLzjOyS6bQ9QsF7yDCKVwiEUtB_RwPFwLmQg@mail.gmail.com>
<CA+HiwqHRRFQN6yZ54fBydOTM6ncqZBCmewZ6n519RjRdDsO44g@mail.gmail.com>
<[email protected]>
<CA+HiwqH8N-SxEB6SysEBsYNgV_KJs66k9Z2SNmqVzbBP-60yWg@mail.gmail.com>
<[email protected]>
<CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
On Thu, Dec 5, 2024 at 3:53 PM Amit Langote <[email protected]> wrote:
> On Thu, Dec 5, 2024 at 2:20 AM Tomas Vondra <[email protected]> wrote:
> > Sure, changing the APIs is allowed, I'm just wondering if maybe there
> > might be a way to not have this issue, or at least notice the missing
> > call early.
> >
> > I haven't tried, wouldn't it be better to modify ExecutorStart() to do
> > the retries internally? I mean, the extensions wouldn't need to check if
> > the plan is still valid, ExecutorStart() would take care of that. Yeah,
> > it might need some new arguments, but that's more obvious.
>
> One approach could be to move some code from standard_ExecutorStart()
> into ExecutorStart(). Specifically, the code responsible for setting
> up enough state in the EState to perform ExecDoInitialPruning(), which
> takes locks that might invalidate the plan. If the plan does become
> invalid, the hook and standard_ExecutorStart() are not called.
> Instead, the caller, ExecutorStartExt() in this case, creates a new
> plan.
>
> This avoids the need to add ExecPlanStillValid() checks anywhere,
> whether in core or extension code. However, it does mean accessing the
> PlannedStmt earlier than InitPlan(), but the current placement of the
> code is not exactly set in stone.
I tried this approach and found that it essentially disables testing
of this patch using the delay_execution module, which relies on the
ExecutorStart_hook(). The way the testing works is that the hook in
delay_execution.c pauses the execution of a cached plan to allow a
concurrent session to drop an index referenced in the plan. When
unpaused, execution initialization resumes by calling
standard_ExecutorStart(). At this point, obtaining the lock on the
partition whose index has been dropped invalidates the plan, which the
hook detects and reports. It then also reports the successful
re-execution of an updated plan that no longer references the dropped
index. Hmm.
--
Thanks, Amit Langote
view thread (66+ 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], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@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