public inbox for [email protected]
help / color / mirror / Atom feedFrom: torikoshia <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: RFC: Logging plan of the running query
Date: Wed, 18 Oct 2023 15:09:38 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAExHW5s+T0Ds-h85y92MsryLqup8fuAT8bovgGGaNnLt=OrQOg@mail.gmail.com>
References: <[email protected]>
<CAAaqYe9TPyWe9yoOm5Jz_39gSnhCAYXozc698jkyegM1x74Qjg@mail.gmail.com>
<CAAaqYe9Mse7JC78DgT97sXoB6U+E5OL3zJAYRSsgy-TPP9PXmw@mail.gmail.com>
<[email protected]>
<CAAaqYe_5p_MkBCgSYX=S7G3ZKi=2=_6bZQFzTgVYyv=1NRikpw@mail.gmail.com>
<[email protected]>
<CAAaqYe_hyWLs-ObFQfu54yA-ZOjNCd-L_Z=1TnYjYNqrLuGiXQ@mail.gmail.com>
<[email protected]>
<CAAaqYe_p-849MtsVgN9_y-YL-uFzLFMpGw38T3Omb11gw-=K1w@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAExHW5ujL=mzbjxf6xtyU-Ft6LMBMEmt8H4ZSDiiZre0Fef0jg@mail.gmail.com>
<[email protected]>
<CAExHW5s+T0Ds-h85y92MsryLqup8fuAT8bovgGGaNnLt=OrQOg@mail.gmail.com>
On 2023-10-16 18:46, Ashutosh Bapat wrote:
> On Thu, Oct 12, 2023 at 6:51 PM torikoshia <[email protected]>
> wrote:
>>
>> On 2023-10-11 16:22, Ashutosh Bapat wrote:
>> >
>> > Considering the similarity with auto_explain I wondered whether this
>> > function should be part of auto_explain contrib module itself? If we
>> > do that users will need to load auto_explain extension and thus
>> > install executor hooks when this function doesn't need those. So may
>> > not be such a good idea. I didn't see any discussion on this.
>>
>> I once thought about adding this to auto_explain, but I left it asis
>> for
>> below reasons:
>>
>> - One of the typical use case of pg_log_query_plan() would be
>> analyzing
>> slow query on customer environments. On such environments, We cannot
>> always control what extensions to install.
>
> The same argument applies to auto_explain functionality as well. But
> it's not part of the core.
Yeah, and when we have a situation where we want to run
pg_log_query_plan(), we can run it in any environment as long as it is
bundled with the core.
On the other hand, if it is built into auto_explain, we need to start by
installing auto_explain if we do not have auto_explain, which is often
difficult to do in production environments.
>> Of course auto_explain is a major extension and it is quite
>> possible
>> that they installed auto_explain, but but it is also possible they do
>> not.
>> - It seems a bit counter-intuitive that pg_log_query_plan() is in an
>> extension called auto_explain, since it `manually`` logs plans
>>
>
> pg_log_query_plan() may not fit auto_explain but
> pg_explain_backend_query() does. What we are logging is more than just
> plan of the query, it might expand to be closer to explain output.
> While auto in auto_explain would refer to its automatically logging
> explain outputs, it can provide an additional function which provides
> similar functionality by manually triggering it.
>
> But we can defer this to a committer, if you want.
>
> I am more interested in avoiding the duplication of code, esp. the
> first comment in my reply
If there are no objections, I will try porting it to auto_explain and
see its feasibility.
>>> There is a lot of similarity between what this feature does and what
>>> auto explain does. I see the code is also duplicated. There is some
>>> merit in avoiding this duplication
>>> 1. we will get all the features of auto_explain automatically like
>>> choosing a format (this was expressed somebody earlier in this
>>> thread), setings etc.
>>> 2. avoid bugs. E.g your code switches context after ExplainState has
>>> been allocated. These states may leak depending upon when this
>>> function gets called.
>>> 3. Building features on top as James envisions will be easier.
>
>>
>> =# select pg_log_query_plan(pid), application_name, backend_type
>> from
>> pg_stat_activity where backend_type = 'autovacuum launcher';
>> WARNING: PID 63323 is not a PostgreSQL client backend process
>> pg_log_query_plan | application_name | backend_type
>> -------------------+------------------+---------------------
>> f | | autovacuum launcher
>>
>>
>> > I am also wondering whether it's better to report the WARNING as
>> > status column in the output. E.g. instead of
>> > #select pg_log_query_plan(100);
>> > WARNING: PID 100 is not a PostgreSQL backend process
>> > pg_log_query_plan
>> > -------------------
>> > f
>> > (1 row)
>> > we output
>> > #select pg_log_query_plan(100);
>> > pg_log_query_plan | status
>> > -------------------+---------------------------------------------
>> > f | PID 100 is not a PostgreSQL backend process
>> > (1 row)
>> >
>> > That looks neater and can easily be handled by scripts, applications
>> > and such. But it will be inconsistent with other functions like
>> > pg_terminate_backend() and pg_log_backend_memory_contexts().
>>
>> It seems neater, but it might be inconvenient because we can no longer
>> use it in select list like the following query as you wrote:
>>
>> #select pg_log_query_plan(pid), application_name, backend_type from
>> pg_stat_activity where backend_type = 'autovacuum launcher';
>
> Why is that?
Sorry, I misunderstood and confirmed we can run queries like below:
```
=# CREATE OR REPLACE FUNCTION pg_log_query_plan_stab(i int)
RETURNS TABLE(
pg_log_query_plan bool,
status text
) AS $$
DECLARE
BEGIN
RETURN QUERY SELECT false::bool, 'PID 100 is not a PostgreSQL
backend process'::text; END;
$$ LANGUAGE plpgsql;
=# select pg_log_query_plan_stab(pid), application_name, backend_type
from pg_stat_activity where backend_type = 'autovacuum launcher';
pg_log_query_plan_stab | application_name |
backend_type
---------------------------------------------------+------------------+---------------------
(f,"PID 100 is not a PostgreSQL backend process") | |
autovacuum launcher
```
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
view thread (25+ 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]
Subject: Re: RFC: Logging plan of the running query
In-Reply-To: <[email protected]>
* 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