Subject: Re: RFC: Logging plan of the running query
To: torikoshia <torikoshia@oss.nttdata.com>,
 Dilip Kumar <dilipbalaut@gmail.com>,
 Pgsql Hackers <pgsql-hackers@postgresql.org>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
References: <cf8501bcd95ba4d727cbba886ba9eea8@oss.nttdata.com>
 <CAFiTN-vE4Q6=bJ-n9hNz8qwkaw2NK1scNC5LrSFkREex+XymLg@mail.gmail.com>
 <CALj2ACXDiOhAdXks_w2b5NGg=7_K27Lx1Gau_iQnxGBCNkSMnQ@mail.gmail.com>
 <CALj2ACXpvVeLstnNG1Oris0VpxbSdLNdh_r-v6YxTEY5vcDHAA@mail.gmail.com>
 <CAFiTN-tbi83fB0j0S7-kj9TraUkim03M3S+oXdYe6sMZ7nxVYA@mail.gmail.com>
 <CALj2ACVH-fhO5htnM2UbV7mvP3+0+zYHBamCTC8KWTtSb5+8=g@mail.gmail.com>
 <CAFiTN-vQcDtyc5aRF1RNo-VgRzMcZ4E1qKTybvHpzUV1FEjqBA@mail.gmail.com>
 <CALj2ACWiVHpOqEztxHRtUR1oiDzPGUnguqv2EfEmQSv7jH93aw@mail.gmail.com>
 <CAFiTN-vOTFRK1xoAWRAGJ+54mSpDxmenAFexbySRhgy7hNH5Fg@mail.gmail.com>
 <CAFiTN-u6yqaxQyfX0jUMqDZ22fePfuQuQmzuozTxC34ca+ZByg@mail.gmail.com>
 <c6682a25f3f0e9bd520707342219eac5@oss.nttdata.com>
 <0842b5da697a03869fb50be93817e4e1@oss.nttdata.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <5ab22d89-a9b9-bcc0-7e91-d4ce7935d050@oss.nttdata.com>
Date: Wed, 9 Jun 2021 23:04:22 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <0842b5da697a03869fb50be93817e4e1@oss.nttdata.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/5ab22d89-a9b9-bcc0-7e91-d4ce7935d050%40oss.nttdata.com>
Precedence: bulk


On 2021/06/09 16:44, torikoshia wrote:
> Updated the patch.

Thanks for updating the patch!

auto_explain can log the plan of even nested statement
if auto_explain.log_nested_statements is enabled.
But ISTM that pg_log_current_plan() cannot log that plan.
Is this intentional?
I think that it's better to make pg_log_current_plan() log
the plan of even nested statement.


+	es->format = EXPLAIN_FORMAT_TEXT;
+	es->settings = true;

Since pg_log_current_plan() is usually used to investigate and
trouble-shoot the long running queries, IMO it's better to
enable es->verbose by default and get additional information
about the queries. Thought?


+ * pg_log_current_plan
+ *		Signal a backend process to log plan the of running query.

"plan the of" is typo?


+ * Only superusers are allowed to signal to log plan because any users to
+ * issue this request at an unbounded rate would cause lots of log messages
+ * and which can lead to denial of service.

"because any users" should be "because allowing any users"
like the comment for pg_log_backend_memory_contexts()?


+ * All the actual work is deferred to ProcessLogExplainInterrupt(),

"ProcessLogExplainInterrupt()" should be "ProcessLogCurrentPlanInterrupt()"?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION