From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Robert Haas <robertmhaas@gmail.com>
To: Fujii Masao <fujii@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Add function to log the memory contexts of specified backend pro
Date: Thu, 1 May 2025 16:53:41 +0900
Message-ID: <ce64a21b-9764-4782-ac94-58703ad6b258@oss.nttdata.com> (raw)
In-Reply-To: <CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com>
References: <E1lTdaH-0002Zo-DS@gemulon.postgresql.org>
<CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com>
On 2025/05/01 2:15, Robert Haas wrote:
> On Tue, Apr 6, 2021 at 12:45 AM Fujii Masao <fujii@postgresql.org> wrote:
>> Add function to log the memory contexts of specified backend process.
>
> Hi,
>
> I think this might need a recursion guard. I tried this:
>
> diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
> index dc4c600922d..b219a934034 100644
> --- a/src/backend/tcop/postgres.c
> +++ b/src/backend/tcop/postgres.c
> @@ -3532,7 +3532,7 @@ ProcessInterrupts(void)
> if (ParallelMessagePending)
> ProcessParallelMessages();
>
> - if (LogMemoryContextPending)
> + if (true)
> ProcessLogMemoryContextInterrupt();
>
> if (PublishMemoryContextPending)
> diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
> index 72f5655fb34..867fd7b0ad5 100644
> --- a/src/include/miscadmin.h
> +++ b/src/include/miscadmin.h
> @@ -112,7 +112,7 @@ extern void ProcessInterrupts(void);
> /* Test whether an interrupt is pending */
> #ifndef WIN32
> #define INTERRUPTS_PENDING_CONDITION() \
> - (unlikely(InterruptPending))
> + (unlikely(InterruptPending) || true)
> #else
> #define INTERRUPTS_PENDING_CONDITION() \
> (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
> pgwin32_dispatch_queued_signals() : 0, \
>
> That immediately caused infinite recursion, ending in a core dump:
>
> frame #13: 0x0000000104607b00
> postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
> funcname=<unavailable>) at elog.c:543:2 [opt]
> frame #14: 0x0000000104637078
> postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
> frame #15: 0x00000001044a901c postgres`ProcessInterrupts at
> postgres.c:3536:3 [opt]
> frame #16: 0x0000000104607b54
> postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
> funcname=<unavailable>) at elog.c:608:2 [opt] [artificial]
> frame #17: 0x0000000104637078
> postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
> frame #18: 0x00000001044a901c postgres`ProcessInterrupts at
> postgres.c:3536:3 [opt]
> <repeat until we have 174241 frames on the stack, then dump core>
>
> It might be unlikely that a process can be signalled fast enough to
> actually fail in this way, but I'm not sure it's impossible, and I
> think we should be defending against it. The most trivial recursion
> guard would be HOLD_INTERRUPTS()/RESUME_INTERRUPTS() around
> ProcessLogMemoryContextInterrupt(), but I think that's probably not
> quite good enough because it would make the backend impervious to
> pg_terminate_backend() while it's dumping memory contexts, and that
> could be a long time if the write blocks.
Just idea, what do you think about adding a flag to indicate whether
ProcessLogMemoryContextInterrupt() is currently running? Then,
when a backend receives a signal and ProcessLogMemoryContextInterrupt()
is invoked, it can simply return immediately if the flag is already set
like this:
------------------------------
@ -1383,8 +1383,14 @@ HandleGetMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool loggingMemoryContext = false;
+
LogMemoryContextPending = false;
+ if (loggingMemoryContext)
+ return;
+ loggingMemoryContext = true;
+
/*
* Use LOG_SERVER_ONLY to prevent this message from being sent to the
* connected client.
@@ -1406,6 +1412,8 @@ ProcessLogMemoryContextInterrupt(void)
* details about individual siblings beyond 100 will not be large.
*/
MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+
+ loggingMemoryContext = false;
}
------------------------------
This way, we can safely ignore overlapping
pg_log_backend_memory_contexts() requests while the function
is already running. Thoughts?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, robertmhaas@gmail.com, fujii@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add function to log the memory contexts of specified backend pro
In-Reply-To: <ce64a21b-9764-4782-ac94-58703ad6b258@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox