Re: partition routing layering in nodeModifyTable.c

On Tue, Oct 27, 2020 at 10:23 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> On 23/10/2020 12:37, Amit Langote wrote:
> > To explain these numbers a bit, "overheaul update/delete processing"
> > patch improves the performance of that benchmark by allowing the
> > updates to use run-time pruning when executing generic plans, which
> > they can't today.
> >
> > However without "lazy-ResultRelInfo-initialization" patch,
> > ExecInitModifyTable() (or InitPlan() when I ran those benchmarks) can
> > be seen to be spending time initializing all of those result
> > relations, whereas only one of those will actually be used.
> >
> > As mentioned further in that email, it's really the locking of all
> > relations by AcquireExecutorLocks() that occurs even before we enter
> > the executor that's a much thornier bottleneck for this benchmark.
> > But the ResultRelInfo initialization bottleneck sounded like it could
> > get alleviated in a relatively straightforward manner. The patches
> > that were developed for attacking the locking bottleneck would require
> > further reflection on whether they are correct.
> >
> > (Note: I've just copy pasted the numbers I reported in that email. To
> > reproduce, I'll have to rebase the "overhaul update/delete processing"
> > patch on this one, which I haven't yet done.)
>
> Ok, thanks for the explanation, now I understand.
>
> But since this applies on top of the "overhaul update/delete processing"
> patch, let's tackle that patch set next. Could you rebase that, please?

Actually, I made lazy-ResultRelInfo-initialization apply on HEAD
directly at one point because of its separate CF entry, that is, to
appease the CF app's automatic patch tester that wouldn't know to
apply the other patch first. Because both of these patch sets want to
change thow ModifyTable works, there are conflicts.

The "overhaul update/delete processing" patch is somewhat complex and
I expect some amount of back and forth on its design points. OTOH,
the lazy-ResultRelInfo-initialization patch is straightforward enough
that I hoped it would be easier to bring it into a committable state
than the other. But I can see why one may find it hard to justify
committing the latter without the former already in, because the
bottleneck it purports to alleviate (that of eager ResultRelInfo
initialization) is not apparent until update/delete can use run-time
pruning.

Anyway, I will post the rebased patch on the "overhaul update/delete
processing" thread.

--
Amit Langote
EDB: http://www.enterprisedb.com