Re: RFC: Logging plan of the running query

On 2021-11-02 20:32, Ekaterina Sokolova wrote:
Thanks for your response!

> Hi!
>
> I'm here to answer your questions about contrib/pg_query_state.
>>> I only took a quick look at pg_query_state, I have some questions.
>
>>> pg_query_state seems using shm_mq to expose the plan information, but
>>> there was a discussion that this kind of architecture would be tricky
>>> to do properly [1].
>>> Does pg_query_state handle difficulties listed on the discussion?
>> [1]
>> https://www.postgresql.org/message-id/9a50371e15e741e295accabc72a41df1%40oss.nttdata.com
>
> I doubt that it was the right link.

Sorry for make you confused, here is the link.

https://www.postgresql.org/message-id/CA%2BTgmobkpFV0UB67kzXuD36--OFHwz1bs%3DL_6PZbD4nxKqUQMw%40mail.gmail.com

> But on the topic I will say that extension really use shared memory,
> interaction is implemented by sending / receiving messages. This
> architecture provides the required reliability and convenience.

As described in the link, using shared memory for this kind of work
would need DSM and It'd be also necessary to exchange information
between requestor and responder.

For example, when I looked at a little bit of pg_query_state code, it
looks like the size of the queue is fixed at QUEUE_SIZE, and I wonder
how plans that exceed QUEUE_SIZE are handled.

>>> It seems the caller of the pg_query_state() has to wait until the
>>> target process pushes the plan information into shared memory, can it
>>> lead to deadlock situations?
>>> I came up with this question because when trying to make a view for
>>> memory contexts of other backends, we encountered deadlock
>>> situations.
>>> After all, we gave up view design and adopted sending signal and
>>> logging.
>>
>> Discussion at the following URL.
>> https://www.postgresql.org/message-id/9a50371e15e741e295accabc72a41df1%40oss.nttdata.com
>
> Before extracting information about side process we check its state.
> Information will only be retrieved for a process willing to provide
> it. Otherwise, we will receive an error message about impossibility of
> getting query execution statistics + process status. Also checking
> fact of extracting your own status exists. This is even verified in
> tests.
>
> Thanks for your attention.
> Just in case, I am ready to discuss this topic in more detail.

I imagined the following procedure.
Does it cause dead lock in pg_query_state?

- session1
BEGIN; TRUNCATE t;

- session2
BEGIN; TRUNCATE t; -- wait

- session1
SELECT * FROM pg_query_state(<pid of session>); -- wait and dead locked?

> About overhead:
>> I haven't measured it yet, but I believe that the overhead for
>> backends
>> which are not called pg_log_current_plan() would be slight since the
>> patch just adds the logic for saving QueryDesc on ExecutorRun().
>> The overhead for backends which is called pg_log_current_plan() might
>> not slight, but since the target process are assumed dealing with
>> long-running query and the user want to know its plan, its overhead
>> would be worth the cost.
> I think it would be useful for us to have couple of examples with a
> different number of rows compared to using without this functionality.

Do you have any expectaion that the number of rows would affect the
performance of this functionality?
This patch adds some codes to ExecutorRun(), but I thought the number of
rows would not give impact on the performance.

--
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION