Re: RFC: Logging plan of the running query

From: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Étienne BERSAC <etienne(dot)bersac(at)dalibo(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, jtc331(at)gmail(dot)com, ashutosh(dot)bapat(dot)oss(at)gmail(dot)com, rafaelthca(at)gmail(dot)com
Subject: Re: RFC: Logging plan of the running query
Date: 2024-01-29 13:02:29
Message-ID: 61b4add9a01f21011789b6ee04085751@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Updated the patch to fix typos and move
ProcessLogQueryPlanInterruptActive from errfinish() to AbortTransaction.

BTW since the thread is getting long, I list the some points of the
discussion so far:

# Safety concern
## Catalog access inside CFI
- it seems safe if the CFI call is inside an existing valid
transaction/query state[1]

- We did some tests, for example calling ProcessLogQueryPlanInterrupt()
in every single CHECK_FOR_INTERRUPTS()[2]. This test passed on my env
but was stucked on James's env, so I modified to exit
ProcessLogQueryPlanInterrupt() when target process is inside of lock
acquisition code[3]

## Risk of calling EXPLAIN code in CFI
- EXPLAIN is not a simple logic code, and there exists risk calling it
from CFI. For example, if there is a bug, we may find ourselves in a
situation where we can't cancel the query

- it's a trade-off that's worth making for the introspection benefits
this patch would provide?[4]

# Design
- Although some suggested it should be in auto_explain, current patch
introduces this feature to the core[5]

- When the target query is nested, only the most inner query's plan is
explained. In future, all the nested queries' plans are expected to
explained optionally like auto_explain.log_nested_statements[6]

- When the target process is a parallel worker, the plan is not shown[6]

- When the target query is nested and its subtransaction is aborted,
pg_log_query_plan cannot log the parental query plan after the abort
even parental query is running[7]

- The output corresponds to EXPLAIN with VERBOSE, COST, SETTINGS and
FORMAT text. It doesn't do ANALYZE or show the progress of the query
execution. Future work proposed by Rafael Thofehrn Castro may realize
this[8]

- To prevent assertion error, this patch ensures no page lock is held by
checking all the LocalLock entries before running explain code, but
there is a discussion that ginInsertCleanup() should be modified[9]

It may be not so difficult to improve some of restrictions in "Design",
but I'd like to limit the scope of the 1st patch to make it simpler.

[1]
https://www.postgresql.org/message-id/CAAaqYe9euUZD8bkjXTVcD9e4n5c7kzHzcvuCJXt-xds9X4c7Fw%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAAaqYe8LXVXQhYy3yT0QOHUymdM%3Duha0dJ0%3DBEPzVAx2nG1gsw%40mail.gmail.com
[3]
https://www.postgresql.org/message-id/0e0e7ca08dff077a625c27a5e0c2ef0a%40oss.nttdata.com
[4]
https://www.postgresql.org/message-id/CAAaqYe8LXVXQhYy3yT0QOHUymdM%3Duha0dJ0%3DBEPzVAx2nG1gsw%40mail.gmail.com
[5]
https://www.postgresql.org/message-id/CAAaqYe_1EuoTudAz8mr8-qtN5SoNtYRm4JM2J9CqeverpE3B2A%40mail.gmail.com
[6]
https://www.postgresql.org/message-id/CAExHW5sh4ahrJgmMAGfptWVmESt1JLKCNm148XVxTunRr%2B-6gA%40mail.gmail.com
[7]
https://www.postgresql.org/message-id/3d121ed5f81cef588bac836b43f5d1f9%40oss.nttdata.com
[8]
https://www.postgresql.org/message-id/c161b5e7e1888eb9c9eb182a7d9dcf89%40oss.nttdata.com
[9]
https://www.postgresql.org/message-id/20220201.172757.1480996662235658750.horikyota.ntt%40gmail.com

--
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation

Attachment Content-Type Size
v35-0001-Add-function-to-log-the-plan-of-the-query.patch text/x-diff 30.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2024-01-29 13:13:21 separating use of SerialSLRULock
Previous Message Alexander Pyhalov 2024-01-29 12:43:34 Re: CREATE INDEX CONCURRENTLY on partitioned index