From: | "Lepikhov Andrei" <a(dot)lepikhov(at)postgrespro(dot)ru> |
---|---|
To: | torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, "James Coleman" <jtc331(at)gmail(dot)com> |
Cc: | "'Andres Freund'" <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, "Greg Stark" <stark(at)mit(dot)edu>, "Ronan Dunklau" <ronan(dot)dunklau(at)aiven(dot)io>, david(dot)christensen(at)crunchydata(dot)com, "Heikki Linnakangas" <hlinnaka(at)iki(dot)fi> |
Subject: | Re: RFC: Logging plan of the running query |
Date: | 2023-09-15 06:21:27 |
Message-ID: | b1b110ae-61f6-4fd9-9b94-f967db9b53d4@app.fastmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 7, 2023, at 1:09 PM, torikoshia wrote:
> On 2023-09-06 11:17, James Coleman wrote:
> It seems that we can know what queries were running from the stack
> traces you shared.
> As described above, I suspect a lock which was acquired prior to
> ProcessLogQueryPlanInterrupt() caused the issue.
> I will try a little more to see if I can devise a way to create the same
> situation.
Hi,
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?
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?
Also, I suggest minor code change with the diff in attachment.
--
Regards,
Andrei Lepikhov
Attachment | Content-Type | Size |
---|---|---|
improve.diff | application/octet-stream | 772 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2023-09-15 07:15:24 | Re: make add_paths_to_append_rel aware of startup cost |
Previous Message | Kyotaro Horiguchi | 2023-09-15 06:17:50 | Re: Bug fix for psql's meta-command \ev |