Re: RFC: Logging plan of the running query

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

In response to

Responses

Browse pgsql-hackers by date

  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