| From: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
| Cc: | Lukas Fittl <lukas(at)fittl(dot)com>, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Dian Fay <di(at)nmfay(dot)com>, Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pg_plan_advice |
| Date: | 2026-01-20 09:19:48 |
| Message-ID: | CAKZiRmxmhjnKp-GMvdL-2jE-nic5g7-TLdsveirrbRv2zdSdyQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Jan 19, 2026 at 9:00 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Jan 19, 2026 at 5:53 AM Jakub Wartak
> <jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
> > a) v10-0001 - any example producing such a dummy subplan? (whatever
> > I've tried I cannot come up with one)
[..]
> EXPLAIN SELECT * FROM random() UNION SELECT * FROM random() WHERE false;
Oh well, that was easy, thanks! Now I see `RTI 3 (function,
in-from-clause): / Subplan: setop_2 (dummy)`
I don't have any further insights on v10-[124] other than mentioned earlier.
> > c) In v10-0004, maybe in pathnodes.h we could use typedef enum rather than
> > list of #defines? (see attached)
>
> I personally hate that style and I think Andres loves it. Whee!
Oh, ok, nvm, but while two of You we are at this, vim or emacs ? ;)
/me ducks & covers
> > 4. Some raw perf numbers on non-assert builds (please ignore +/- 3%
> > jumps), it just hurts
> > in one scenario where oq2 drops like 9% of juice (quite expected, it's not
> > an issue to be, just posting full results)
> >
> > tps oq1 oq2 oq3 oq4
> > master 41 14745 439 435
> > master+v10-000[1-4] 42 15055 439 432
> > master+v10full 41 14734 429 437
> > master+v10full+loaded 42 15014 442 438
> > master+v10full+loaded+advice 41 13481 424 439
> >
> > (same but in percentages)
> > %tps_to_master oq1 oq2 oq3 oq4
> > master 100 100 100 100
> > master+v10-000[1-4] 102 102 100 99
> > master+v10full 100 100 98 100
> > master+v10full+loaded 102 102 101 101
> > master+v10full+loaded+advice 100 91 97 101
>
> I think these numbers look pretty good. I mean, there is obviously
> room for improvement. We should look at where the CPU cycles are going
> in the oq2 case and try to optimize. But even without that, it's not
> terrible, IMHO.
>
> > So out of curiosity the oq2 on 1 CPU core behavior looks like below:
> > - no advices --> ~1000 TPS
> > - enabled pg_plan_advice.advice to lengthy, but unrelated thing and it
> > gets ~890TPS
>
> I'm not sure exactly where the CPU cycles are going here, but one
> known problem is that we have to re-parse the advice string for every
> query. This thread discusses the challenges of creating some
> infrastructure that would allow us to avoid that:
>
> http://postgr.es/m/f87504a6-9dfd-4467-89de-84232cb54f72@gmail.com
>
> Maybe I should start thinking about other ways to avoid that overhead,
Meh...
> because that thread doesn't seem to be progressing much, but maybe the
> reparsing isn't even the main problem.
> > - in both cases (empty and set) the bottleneck seems to in palloc0, but
> > empty plan_advice: it's more like palloc0() <- newNode() <-
> > create_index_path()
> > <- build_index_paths()
> > with plan_advice set: palloc0() <- newNode() <- create_nestloop_path() ..
>
> I've also seen some palloc-related issues with the patch -- it has to
> build some data structures and that does palloc stuff -- but there
> shouldn't really be any difference in the code paths you show here.
> That's just core code, which should be doing the same thing either way
> if the advice is not relevant.
Yeah, in both it looks like memory allocation and lots of newNode()
called , quite expected.
> > - so if anything people should not put something there blindly, but just SET
> > and RESET afterwards (unless we get pinning of SQL plan id to advices) as
> > this might have cost in high-TPS scenarios.
>
> Yes, I think that's definitely a potential issue. I'd like the
> overhead of this module to be as low as possible, but it's bound to
> have at least some overhead, and people will have to decide whether
> it's worth it.
I think we should simply ignore, and maybe later just note the fact this is
not free with a single sentence in some docs for 0005. I was just curious of the
impact and this was measured using pure EXPLAIN (so without query execution to
measure impact of non-empty pg_plan_advice), I'm assuming that in
properly managed
systems execution part will always dominate the workload anyway and
one should be
using prepared statements anyway.
-J.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-01-20 09:28:50 | Re: Periodic authorization expiration checks using GoAway message |
| Previous Message | Japin Li | 2026-01-20 09:02:19 | Re: Add IS_INDEX macro to brin and gist index |