Re: pg_log_backend_memory_contexts() and log level
On 2022/01/27 12:45, Fujii Masao wrote: Thanks for the review! So barring any objection, I will commit the patch and backport it to v14 where pg_log_backend_memory_contexts() is added. Pushed. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: pg_log_backend_memory_contexts() and log level
On 2022/01/26 14:28, torikoshia wrote: On 2022-01-26 13:17, Fujii Masao wrote: Hi, pg_log_backend_memory_contexts() should be designed not to send the messages about the memory contexts to the client regardless of client_min_messages. But I found that the message "logging memory contexts of PID %d" can be sent to the client because it's ereport()'d with LOG level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't we use LOG_SERVER_ONLY level to log that message? Patch attached. Regards, Thanks! I think it's a bug. There are two clients: "the client that executed pg_log_backend_memory_contexts()" and "the client that issued the query that was the target of the memory context logging", but I was only concerned about the former when I wrote that part of the code. The latter is not expected. It seems better to use LOG_SERVER_ONLY as the attached patch. The patch was successfully applied and there was no regression test error on my environment. Thanks for the review! So barring any objection, I will commit the patch and backport it to v14 where pg_log_backend_memory_contexts() is added. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: pg_log_backend_memory_contexts() and log level
On 2022/01/26 14:27, Bharath Rupireddy wrote:
On Wed, Jan 26, 2022 at 10:46 AM Bharath Rupireddy
wrote:
On Wed, Jan 26, 2022 at 9:48 AM Fujii Masao wrote:
Hi,
pg_log_backend_memory_contexts() should be designed not to send the messages about the
memory contexts to the client regardless of client_min_messages. But I found that the
message "logging memory contexts of PID %d" can be sent to the client because
it's ereport()'d with LOG level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't
we use LOG_SERVER_ONLY level to log that message? Patch attached.
+1. The patch LGTM.
Thanks for the review!
While we are here, I think it's enough to have the
has_function_privilege, not needed to set role regress_log_memory and
then execute the function as we already have code covering the
function above (SELECT
pg_log_backend_memory_contexts(pg_backend_pid());). Thought?
SELECT has_function_privilege('regress_log_memory',
'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- yes
SET ROLE regress_log_memory;
SELECT pg_log_backend_memory_contexts(pg_backend_pid());
RESET ROLE;
Or it's enough to set role and execute the function? Either those or
has_function_privilege() can be removed from the test, but I don't have strong
reason to do that for now...
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Re: pg_log_backend_memory_contexts() and log level
On 2022-01-26 13:17, Fujii Masao wrote: Hi, pg_log_backend_memory_contexts() should be designed not to send the messages about the memory contexts to the client regardless of client_min_messages. But I found that the message "logging memory contexts of PID %d" can be sent to the client because it's ereport()'d with LOG level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't we use LOG_SERVER_ONLY level to log that message? Patch attached. Regards, Thanks! I think it's a bug. There are two clients: "the client that executed pg_log_backend_memory_contexts()" and "the client that issued the query that was the target of the memory context logging", but I was only concerned about the former when I wrote that part of the code. The latter is not expected. It seems better to use LOG_SERVER_ONLY as the attached patch. The patch was successfully applied and there was no regression test error on my environment. -- Regards, -- Atsushi Torikoshi NTT DATA CORPORATION
Re: pg_log_backend_memory_contexts() and log level
On Wed, Jan 26, 2022 at 10:46 AM Bharath Rupireddy
wrote:
>
> On Wed, Jan 26, 2022 at 9:48 AM Fujii Masao
> wrote:
> >
> > Hi,
> >
> > pg_log_backend_memory_contexts() should be designed not to send the
> > messages about the memory contexts to the client regardless of
> > client_min_messages. But I found that the message "logging memory contexts
> > of PID %d" can be sent to the client because it's ereport()'d with LOG
> > level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't we use
> > LOG_SERVER_ONLY level to log that message? Patch attached.
>
> +1. The patch LGTM.
While we are here, I think it's enough to have the
has_function_privilege, not needed to set role regress_log_memory and
then execute the function as we already have code covering the
function above (SELECT
pg_log_backend_memory_contexts(pg_backend_pid());). Thought?
SELECT has_function_privilege('regress_log_memory',
'pg_log_backend_memory_contexts(integer)', 'EXECUTE'); -- yes
SET ROLE regress_log_memory;
SELECT pg_log_backend_memory_contexts(pg_backend_pid());
RESET ROLE;
Regards,
Bharath Rupireddy.
Re: pg_log_backend_memory_contexts() and log level
On Wed, Jan 26, 2022 at 10:46 AM Bharath Rupireddy wrote: > > On Wed, Jan 26, 2022 at 9:48 AM Fujii Masao > wrote: > > > > Hi, > > > > pg_log_backend_memory_contexts() should be designed not to send the > > messages about the memory contexts to the client regardless of > > client_min_messages. But I found that the message "logging memory contexts > > of PID %d" can be sent to the client because it's ereport()'d with LOG > > level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't we use > > LOG_SERVER_ONLY level to log that message? Patch attached. > > +1. The patch LGTM. > > The same applies for the on-going patch [1] for pg_log_backtrace, let > me quickly send the updated patch there as well. > > [1] - > https://www.postgresql.org/message-id/CALDaNm3tbc1OKvSKvD5SfmEj66M_sWDPCgDvzFtS9gxRov8jRQ%40mail.gmail.com My bad, the v17 patch of pg_log_backtrace at [1] does emit at LOG_SERVER_ONLY. Regards, Bharath Rupireddy.
Re: pg_log_backend_memory_contexts() and log level
On Wed, Jan 26, 2022 at 9:48 AM Fujii Masao wrote: > > Hi, > > pg_log_backend_memory_contexts() should be designed not to send the messages > about the memory contexts to the client regardless of client_min_messages. > But I found that the message "logging memory contexts of PID %d" can be sent > to the client because it's ereport()'d with LOG level instead of > LOG_SERVER_ONLY. Is this a bug, and shouldn't we use LOG_SERVER_ONLY level to > log that message? Patch attached. +1. The patch LGTM. The same applies for the on-going patch [1] for pg_log_backtrace, let me quickly send the updated patch there as well. The pg_log_query_plan on-going patch [2], does the right thing. [1] - https://www.postgresql.org/message-id/CALDaNm3tbc1OKvSKvD5SfmEj66M_sWDPCgDvzFtS9gxRov8jRQ%40mail.gmail.com [2] - https://www.postgresql.org/message-id/fb6360e4c0ffbdd0a6698b92bb2617e2%40oss.nttdata.com Regards, Bharath Rupireddy.
pg_log_backend_memory_contexts() and log level
Hi,
pg_log_backend_memory_contexts() should be designed not to send the messages about the
memory contexts to the client regardless of client_min_messages. But I found that the
message "logging memory contexts of PID %d" can be sent to the client because
it's ereport()'d with LOG level instead of LOG_SERVER_ONLY. Is this a bug, and shouldn't
we use LOG_SERVER_ONLY level to log that message? Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATIONdiff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 2876f6868c..e12be1b9bd 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1042,8 +1042,14 @@ ProcessLogMemoryContextInterrupt(void)
{
LogMemoryContextPending = false;
- ereport(LOG,
- (errmsg("logging memory contexts of PID %d",
MyProcPid)));
+ /*
+* Use LOG_SERVER_ONLY to prevent this message from being sent to the
+* connected client.
+*/
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+errhidecontext(true),
+errmsg("logging memory contexts of PID %d",
MyProcPid)));
/*
* When a backend process is consuming huge memory, logging all its
memory
