Re: RFC: Logging plan of the running query

From: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, James Coleman <jtc331(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Étienne BERSAC <etienne(dot)bersac(at)dalibo(dot)com>, ashutosh(dot)bapat(dot)oss(at)gmail(dot)com, rafaelthca(at)gmail(dot)com, jian(dot)universality(at)gmail(dot)com
Subject: Re: RFC: Logging plan of the running query
Date: 2024-02-26 12:01:45
Message-ID: 2c9a3fd390d653f1829b795d0bad4653@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-02-24 00:23, Robert Haas wrote:
> On Fri, Feb 23, 2024 at 7:50 PM Julien Rouhaud <rjuju123(at)gmail(dot)com>
> wrote:
>> On Fri, Feb 23, 2024 at 10:22:32AM +0530, Robert Haas wrote:
>> > On Thu, Feb 22, 2024 at 6:25 AM James Coleman <jtc331(at)gmail(dot)com> wrote:
>> > > This is potentially a bit of a wild idea, but I wonder if having some
>> > > kind of argument to CHECK_FOR_INTERRUPTS() signifying we're in
>> > > "normal" as opposed to "critical" (using that word differently than
>> > > the existing critical sections) would be worth it.
>> >
>> > It's worth considering, but the definition of "normal" vs. "critical"
>> > might be hard to pin down. Or, we might end up with a definition that
>> > is specific to this particular case and not generalizable to others.
>>
>> But it doesn't have to be all or nothing right? I mean each call
>> could say
>> what the situation is like in their context, like
>> CHECK_FOR_INTERRUPTS(GUARANTEE_NO_HEAVYWEIGHT_LOCK |
>> GUARANTEE_WHATEVER), and
>> slowly tag calls as needed, similarly to how we add already CFI based
>> on users
>> report.
>
> Absolutely. My gut feeling is that it's going to be simpler to pick a
> small number of places that are safe and sufficient for this
> particular feature and add an extra call there

Hmm, whether extending CHECK_FOR_INTERRUPTS() or adding extras call
directly, currently I'm not sure where are the good 'places', which
don't give performance impact.

As attached PoC patch, I experimentally added extra calls on
ExecScanFetch() which would be less called than ExecProcNode()[1].
When running sequential scan on pgbench_accounts which is on the memory,
there seems a performance degradation.

- Executed "select * from pgbench_accounts" for 20 times
- Compared the elapsed time between the patch applied and not applied
on 874d817baa160ca7e68
- there were no heap_blks_read during the query
- pgbench_accounts has 3000000 rows

patch NOT applied:
- average: 335.88 ms
- max: 367.313 ms
- min: 309.609 ms

patch applied:
- average: 342.57 ms
- max: 380.099 ms
- min: 324.270 ms

It would be nice if there was a place accessed once every few seconds or
so..

[1]
https://www.postgresql.org/message-id/20240215185911.v4o6fo444md6a3w7%40awork3.anarazel.de

--
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

Attachment Content-Type Size
v37-0001-Add-function-to-log-the-plan-of-the-query.patch text/x-diff 32.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2024-02-26 12:15:53 RE: speed up a logical replica setup
Previous Message Amit Kapila 2024-02-26 11:52:20 Re: Synchronizing slots from primary to standby