Re: RFC: Logging plan of the running query

On 2021-06-15 13:27, Bharath Rupireddy wrote:
> On Mon, Jun 14, 2021 at 5:48 PM torikoshia <torikoshia@oss.nttdata.com> 
> wrote:
>> > 1) We could just say "Requests to log query plan of the presently
>> > running query of a given backend along with an untruncated query
>> > string in the server logs."
>> > Instead of
>> > +        They will be logged at <literal>LOG</literal> message level
>> > and
>> > +        will appear in the server log based on the log
>> > +        configuration set (See <xref
>> > linkend="runtime-config-logging"/>
>> 
>> Actually this explanation is almost the same as that of
>> pg_log_backend_memory_contexts().
>> Do you think we should change both of them?
>> I think it may be too detailed but accurate.
> 
> I withdraw my comment. We can keep the explanation similar to
> pg_log_backend_memory_contexts as it was agreed upon and committed
> text. If the wordings are similar, then it will be easier for users to
> understand the documentation.
> 
>> > 5) Instead of just showing the true return value of the function
>> > pg_log_current_plan in the sql test, which just shows that the signal
>> > is sent, but it doesn't mean that the backend has processed that
>> > signal and logged the plan. I think we can add the test using TAP
>> > framework, one
>> 
>> I once made a tap test for pg_log_backend_memory_contexts(), but it
>> seemed an overkill and we agreed that it was appropriate just ensuring
>> the function working as below discussion.
>> 
>> https://www.postgresql.org/message-id/bbecd722d4f8e261b350186ac4bf68a7%40oss.nttdata.com
> 
> Okay. I withdraw my comment.
> 
>> > 6) Do we unnecessarily need to signal the processes such as autovacuum
>> > launcher/workers, logical replication launcher/workers, wal senders,
>> > wal receivers and so on. only to emit a "PID %d is not executing
>> > queries now" message? Moreover, those processes will be waiting in
>> > loops for timeouts to occur, then as soon as they wake up do they need
>> > to process this extra uninformative signal?
>> > We could choose to not signal those processes at all which might or
>> > might not be possible.
>> > Otherwise, we could just emit messages like "XXXX process cannot run a
>> > query" in ProcessInterrupts.
>> 
>> Agreed.
>> 
>> However it needs to distinguish backends which can execute queries and
>> other processes such as autovacuum launcher, I don't come up with
>> easy ways to do so.
>> I'm going to think about it.
> 
> I'm not sure if there is any information in the shared memory
> accessible to all the backends/sessions that can say a PID is
> autovacuum launcher/worker, logical replication launcher/worker or any
> other background or parallel worker.

As far as I looked around, there seems no easy ways to do so.

> If we were to invent a new
> mechanism just for addressing the above comment, I would rather choose
> to not do that as it seems like an overkill. We can leave it up to the
> user whether or not to unnecessarily signal those processes which are
> bound to print "PID XXX is not executing queries now" message.

+1. I'm going to proceed in this direction.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION