Re: RFC: Logging plan of the running query

From: Alena Rybakina <lena(dot)ribackina(at)yandex(dot)ru>
To: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: RFC: Logging plan of the running query
Date: 2022-09-21 08:30:25
Message-ID: d4a5ea75-d6a3-7f02-890e-8e3d1a43aeab@yandex.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

 Ok, I get it.

> Since GetLockMethodLocalHash() is only used for assertions, this is
> only defined when USE_ASSERT_CHECKING is enabled. This patch uses
> GetLockMethodLocalHash() not only for the assertion purpose, so I
> removed "ifdef USE_ASSERT_CHECKING" for this function. I belive it
> does not lead to skip errors.
Agree.
> Since these processes are needed for nested queries, not only for
> AbortSubTransaction[1], added comments on it.

I also noticed it. However I also discovered that above function
declarations to be aplied for explain command and used to be printed
details of the executed query.

We have a similar task to print the plan of an interrupted process
making a request for a specific pid.

In short, I think, this task is different and for separating these parts
I added this comment.

> I feel this comment is unnecessary since the explanation of
> HandleLogQueryPlanInterrupt() is written in explain.c and no functions
> in explain.h have comments in it.

Yes, I was worried about it. I understood it, thank for explaining.

>
> AFAIU both of them are processed since SendProcSignal flags
> ps_signalFlags for each signal.
>
> ```
> SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId)
> {
> volatile ProcSignalSlot *slot;
> ...(snip)...
> 278 if (slot->pss_pid == pid)
> 279 {
> 280 /* Atomically set the proper flag */
> 281 slot->pss_signalFlags[reason] = true;
> 282 /* Send signal */
> 283 return kill(pid, SIGUSR1);
> ```
>
> Comments of ProcSignalReason also say 'We can cope with concurrent
> signals for different reasons'.
>
> ```C
> /*
> * Reasons for signaling a Postgres child process (a backend or an
> auxiliary
> * process, like checkpointer). We can cope with concurrent signals for
> different
> * reasons. However, if the same reason is signaled multiple times in
> quick
> * succession, the process is likely to observe only one notification
> of it.
> * This is okay for the present uses.
> ...
> typedef enum
> {
> PROCSIG_CATCHUP_INTERRUPT, /* sinval catchup interrupt */
> PROCSIG_NOTIFY_INTERRUPT, /* listen/notify interrupt */
> PROCSIG_PARALLEL_MESSAGE, /* message from cooperating parallel backend */
> PROCSIG_WALSND_INIT_STOPPING, /* ask walsenders to prepare for
> shutdown */
> PROCSIG_BARRIER, /* global barrier interrupt */
> PROCSIG_LOG_MEMORY_CONTEXT, /* ask backend to log the memory contexts */
> PROCSIG_LOG_QUERY_PLAN, /* ask backend to log plan of the current
> query */
> ...
> } ProcSignalReason;
> ```
>
>
> [1]
> https://www.postgresql.org/message-id/8b53b32f-26cc-0531-4ac0-27310e0bef4b%40oss.nttdata.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2022-09-21 09:25:16 Re: Perform streaming logical transactions by background workers and parallel apply
Previous Message wangw.fnst@fujitsu.com 2022-09-21 08:27:58 RE: Perform streaming logical transactions by background workers and parallel apply