public inbox for [email protected]
help / color / mirror / Atom feedFrom: torikoshia <[email protected]>
To: Lepikhov Andrei <[email protected]>
Cc: James Coleman <[email protected]>
Cc: 'Andres Freund' <[email protected]>
Cc: [email protected]
Cc: Greg Stark <[email protected]>
Cc: Ronan Dunklau <[email protected]>
Cc: [email protected]
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: RFC: Logging plan of the running query
Date: Mon, 25 Sep 2023 16:21:31 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAAaqYe9euUZD8bkjXTVcD9e4n5c7kzHzcvuCJXt-xds9X4c7Fw@mail.gmail.com>
<[email protected]>
<CAAaqYe8LXVXQhYy3yT0QOHUymdM=uha0dJ0=BEPzVAx2nG1gsw@mail.gmail.com>
<[email protected]>
<CAAaqYe9u-ywvRX32iBj1SkQrLf1hqMhR9HFCc0dAk4rGDJpyFw@mail.gmail.com>
<[email protected]>
<CAAaqYe-5jKTuuZB5StBwa2YRiqpnN=q9buKADP8A_RPd09f_Ug@mail.gmail.com>
<CAAaqYe-M63VNMpgq0sEw3+t2KaJyhXfu3uBU7w_jtfbGAm4LoA@mail.gmail.com>
<[email protected]>
<CAAaqYe9QB_CUjXrvNh-cpzW6Hz7S4o5pZnR1U1TujLGptUrS1A@mail.gmail.com>
<[email protected]>
<[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]>
<[email protected]>
<[email protected]>
<[email protected]>
On 2023-09-20 14:39, Lepikhov Andrei wrote:
Thanks for your reply.
> On Tue, Sep 19, 2023, at 8:39 PM, torikoshia wrote:
>> On 2023-09-15 15:21, Lepikhov Andrei wrote:
>>> On Thu, Sep 7, 2023, at 1:09 PM, torikoshia wrote:
>>> I have explored this patch and, by and large, agree with the code.
>>> But
>>> some questions I want to discuss:
>>> 1. Maybe add a hook to give a chance for extensions, like
>>> pg_query_state, to do their job?
>>
>> Do you imagine adding a hook which enables adding custom interrupt
>> codes
>> like below?
>>
>> https://github.com/postgrespro/pg_query_state/blob/master/patches/custom_signals_15.0.patch
>
> No, I think around the hook, which would allow us to rewrite the
> pg_query_state extension without additional patches by using the
> functionality provided by your patch. I mean, an extension could
> provide console UI, not only server logging. And obtain from target
> backend so much information in the explain as the instrumentation
> level of the current query can give.
> It may make pg_query_state shorter and more stable.
>
>>> 2. In this implementation, ProcessInterrupts does a lot of work
>>> during
>>> the explain creation: memory allocations, pass through the plan,
>>> systables locks, syscache access, etc. I guess it can change the
>>> semantic meaning of 'safe place' where CHECK_FOR_INTERRUPTS can be
>>> called - I can imagine a SELECT query, which would call a stored
>>> procedure, which executes some DDL and acquires row exclusive lock at
>>> the time of interruption. So, in my mind, it is too unpredictable to
>>> make the explain in the place of interruption processing. Maybe it is
>>> better to think about some hook at the end of ExecProcNode, where a
>>> pending explain could be created?
>>
>> Yeah, I withdrew this patch once for that reason, but we are resuming
>> development in response to the results of a discussion by James and
>> others at this year's pgcon about the safety of this process in CFI:
>>
>> https://www.postgresql.org/message-id/CAAaqYe9euUZD8bkjXTVcD9e4n5c7kzHzcvuCJXt-xds9X4c7Fw%40mail.gma...
>
> I can't track the logic path of the decision provided at this
> conference. But my anxiety related to specific place, where
> ActiveQueryDesc points top-level query and interruption comes during
> DDL, wrapped up in stored procedure. For example:
> CREATE TABLE test();
> CREATE OR REPLACE FUNCTION ddl() RETURNS void AS $$
> BEGIN
> EXECUTE format('ALTER TABLE test ADD COLUMN x integer;');
> ...
> END; $$ LANGUAGE plpgsql VOLATILE;
> SELECT ddl(), ... FROM ...;
Hmm, as a test, I made sure to call ProcessLogQueryPlanInterrupt() on
all CFI using
v28-0002-Testing-attempt-logging-plan-on-ever-CFI-call.patch, and then
ran the following query but did not cause any problems.
```
=# CREATE TABLE test();
=# CREATE OR REPLACE FUNCTION ddl() RETURNS void AS $$
BEGIN
EXECUTE format('ALTER TABLE test ADD COLUMN x integer;');
PERFORM pg_sleep(5);
END; $$ LANGUAGE plpgsql VOLATILE;
=# SELECT ddl();
```
Is this the case you're worrying about?
--
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], [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