Horton

Re: RFC: Logging plan of the running query

torikoshia <torikoshia@oss.nttdata.com>

View thread
From: torikoshia <torikoshia@oss.nttdata.com>
To: James Coleman <jtc331@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org, Greg Stark <stark@mit.edu>, Ronan Dunklau <ronan.dunklau@aiven.io>, david.christensen@crunchydata.com, Heikki Linnakangas <hlinnaka@iki.fi>
Date: 2023-06-12T04:07:34Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. ecpg: Catch zero-length Unicode identifiers correctly

  2. Improve warning message in pg_signal_backend()

  3. Add assert to ensure that page locks don't participate in deadlock cycle.

On 2023-06-06 03:26, James Coleman wrote:
> On Mon, Jun 5, 2023 at 4:30 AM torikoshia <torikoshia@oss.nttdata.com> 
> wrote:
>> 
>> On 2023-06-03 02:51, James Coleman wrote:
>> > Hello,
>> >
>> > Thanks for working on this patch!
> 
> Sure thing! I'm *very interested* in seeing this available, and I
> think it paves the way for some additional features later on...
> 
>> > On Thu, Dec 8, 2022 at 12:10 AM torikoshia <torikoshia@oss.nttdata.com>
>> ...
>> > To put it positively: we believe that, for example, catalog accesses
>> > inside CHECK_FOR_INTERRUPTS() -- assuming that the CFI call is inside
>> > an existing valid transaction/query state, as it would be for this
>> > patch -- are safe. If there were problems, then those problems are
>> > likely bugs we already have in other CFI cases.
>> 
>> Thanks a lot for the discussion and sharing it!
>> I really appreciate it.
>> 
>> BTW I'm not sure whether all the CFI are called in valid transaction,
>> do you think we should check each of them?
> 
> I kicked off the regressions tests with a call to
> ProcessLogQueryPlanInterrupt() in every single CHECK_FOR_INTERRUPTS()
> call. Several hours and 52 GB of logs later I have confirmed that
> (with the attached revision) at the very least the regression test
> suite can't trigger any kind of failures regardless of when we trigger
> this. The existing code in the patch for only running the explain when
> there's an active query handling that.

Thanks for the testing!

> I've attached v27. The important change here in 0001 is that it
> guarantees the interrupt handler is re-entrant, since that was a bug
> exposed by my testing. I've also included 0002 which is only meant for
> testing (it attempts to log in the plan in every
> CHECK_FOR_INTERRUPTS() call).

When SIGINT is sent during ProcessLogQueryPlanInterrupt(),
ProcessLogQueryPlanInterruptActive can remain true.
After that, pg_log_query_plan() does nothing but just returns.

AFAIU, v26 logs plan for each pg_log_query_plan() even when another
pg_log_query_plan() is running, but it doesn't cause errors or critical
problem.

Considering the problem solved and introduced by v27, I think v26 might
be fine.
How do you think?

> 
> Regards,
> James

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION