From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | DIPESH DHAMELIYA <dipeshdhameliya125(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7 |
Date: | 2025-07-05 05:26:19 |
Message-ID: | CAFiTN-vEKk1+GoPZmLTK0cb5PDEtYHHr7NJco3DK=ZEa-mMsvA@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jul 4, 2025 at 9:56 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Dilip Kumar <dilipbalaut(at)gmail(dot)com> writes:
> > On Fri, Jul 4, 2025 at 1:22 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> There's no initplan in the given test case, so I don't see how
> >> that idea is going to fix it. Also, allowing initplans to begin
> >> parallelism when the outer query isn't using parallelism seems
> >> like it'd be fraught with problems. It certainly couldn't be
> >> something we'd back-patch.
>
> > If you see the original problematic case[1] shown by dipesh, where the
> > parallel plan is selected for the initPlan1 but it could not launch
> > the worker because of the below check[2] in ExecutePlan. So here my
> > concern was that the number of "max tuple" was passed for the top
> > level plan, however the parallelism is restricted for the InitPlan as
> > well.
>
> Oh, looking back, I see that indeed the original example happened to
> produce a plan that involves an initplan. But there are plenty of
> variants that won't, such as the arguably-more-idiomatic
>
> return count(*) from test_tab where a between 5.0 and 999999.0;
>
> So I think your emphasis on improving that case is misplaced to start
> with.
>
> > If I understand the code comments correctly, they state that "Parallel
> > mode only supports complete execution of a plan." Given that
> > "InitPlan1" does run to completion, it seems the issue here is that
> > when the top-level plan isn't fully executed, it restricts parallelism
> > for its sub-plans or init-plans, even if those sub-plans are running
> > to completion.
>
> I repeat that I don't think "allow an initplan to run in parallel even
> if the outer query can't" is a particularly sane way to tackle this
> problem. For starters, ExecutorRun calls EnterParallelMode() and
> ExitParallelMode() globally for the entire query. We'd have to
> rethink how that works and when it would be safe to call those.
> Using the query-global estate->es_use_parallel_mode flag wouldn't work
> either. Thought would also be needed about which can't-do-parallelism
> restrictions would be safe to override. I agree that an outer
> numberTuples restriction needn't be considered, but I'm not sure that
> any others could be ignored.
>
> So on the whole it seems like a research project requiring nontrivial
> effort and probably yielding only marginal gains. It's certainly not
> going to yield a back-patchable fix for this performance regression.
Yeah that's correct.
--
Regards,
Dilip Kumar
Google
From | Date | Subject | |
---|---|---|---|
Next Message | Cédric Villemain | 2025-07-05 07:09:00 | Re: Adding basic NUMA awareness - Preliminary feedback and outline for an extensible approach |
Previous Message | Noah Misch | 2025-07-05 04:15:59 | Re: Get rid of WALBufMappingLock |