Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add guard to prevent recursive memory context logging.

  2. Add function to log the memory contexts of specified backend process.

  3. Add pg_backend_memory_contexts system view.

  1. pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <fujii@postgresql.org> — 2021-04-06T04:45:01Z

    Add function to log the memory contexts of specified backend process.

Commit 3e98c0bafb added pg_backend_memory_contexts view to display
the memory contexts of the backend process. However its target process
is limited to the backend that is accessing to the view. So this is
not so convenient when investigating the local memory bloat of other
backend process. To improve this situation, this commit adds
pg_log_backend_memory_contexts() function that requests to log
the memory contexts of the specified backend process.

This information can be also collected by calling
MemoryContextStats(TopMemoryContext) via a debugger. But
this technique cannot be used in some environments because no debugger
is available there. So, pg_log_backend_memory_contexts() allows us to
see the memory contexts of specified backend more easily.

Only superusers are allowed to request to log the memory contexts
because allowing any users to issue this request at an unbounded rate
would cause lots of log messages and which can lead to denial of service.

On receipt of the request, at the next CHECK_FOR_INTERRUPTS(),
the target backend logs its memory contexts at LOG_SERVER_ONLY level,
so that these memory contexts will appear in the server log but not
be sent to the client. It logs one message per memory context.
Because if it buffers all memory contexts into StringInfo to log them
as one message, which may require the buffer to be enlarged very much
and lead to OOM error since there can be a large number of memory
contexts in a backend.

When a backend process is consuming huge memory, logging all its
memory contexts might overrun available disk space. To prevent this,
now this patch limits the number of child contexts to log per parent
to 100. As with MemoryContextStats(), it supposes that practical cases
where the log gets long will typically be huge numbers of siblings
under the same parent context; while the additional debugging value
from seeing details about individual siblings beyond 100 will not be large.

There was another proposed patch to add the function to return
the memory contexts of specified backend as the result sets,
instead of logging them, in the discussion. However that patch is
not included in this commit because it had several issues to address.

Thanks to Tatsuhito Kasahara, Andres Freund, Tom Lane, Tomas Vondra,
Michael Paquier, Kyotaro Horiguchi and Zhihong Yu for the discussion.

Bump catalog version.

Author: Atsushi Torikoshi
Reviewed-by: Kyotaro Horiguchi, Zhihong Yu, Fujii Masao
Discussion: https://postgr.es/m/0271f440ac77f2a4180e0e56ebd944d1@oss.nttdata.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/43620e328617c1f41a2a54c8cee01723064e3ffa

Modified Files
--------------
doc/src/sgml/func.sgml                       |  52 ++++++++
src/backend/storage/ipc/procsignal.c         |   4 +
src/backend/tcop/postgres.c                  |   3 +
src/backend/utils/adt/mcxtfuncs.c            |  60 ++++++++-
src/backend/utils/init/globals.c             |   1 +
src/backend/utils/mmgr/aset.c                |   8 +-
src/backend/utils/mmgr/generation.c          |   8 +-
src/backend/utils/mmgr/mcxt.c                | 180 ++++++++++++++++++++++-----
src/backend/utils/mmgr/slab.c                |   9 +-
src/include/catalog/catversion.h             |   2 +-
src/include/catalog/pg_proc.dat              |   6 +
src/include/miscadmin.h                      |   1 +
src/include/nodes/memnodes.h                 |   6 +-
src/include/storage/procsignal.h             |   1 +
src/include/utils/memutils.h                 |   5 +-
src/test/regress/expected/misc_functions.out |  13 ++
src/test/regress/sql/misc_functions.sql      |   9 ++
17 files changed, 320 insertions(+), 48 deletions(-)

  2. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Robert Haas <robertmhaas@gmail.com> — 2025-04-30T17:15:46Z

    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.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  3. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-01T07:53:41Z

    
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

  4. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Robert Haas <robertmhaas@gmail.com> — 2025-05-01T12:42:05Z

    On Thu, May 1, 2025 at 3:53 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> 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:

I think that something like this could work, but you would need more
than this. Otherwise, if the function errors out, the flag would
remain permanently set.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  5. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-01T17:27:54Z

    
On 2025/05/01 21:42, Robert Haas wrote:
> On Thu, May 1, 2025 at 3:53 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> 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:
> 
> I think that something like this could work, but you would need more
> than this. Otherwise, if the function errors out, the flag would
> remain permanently set.

Yes, we need to either use PG_TRY()/PG_FINALLY() or handle the flag as
a global variable and reset it in the error handling path. I think using
PG_TRY()/PG_FINALLY() is the simpler option.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  6. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-02T00:02:43Z

    
On 2025/05/02 2:27, Fujii Masao wrote:
> 
> 
> On 2025/05/01 21:42, Robert Haas wrote:
>> On Thu, May 1, 2025 at 3:53 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>> 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:
>>
>> I think that something like this could work, but you would need more
>> than this. Otherwise, if the function errors out, the flag would
>> remain permanently set.
> 
> Yes, we need to either use PG_TRY()/PG_FINALLY() or handle the flag as
> a global variable and reset it in the error handling path. I think using
> PG_TRY()/PG_FINALLY() is the simpler option.

I've implemented the patch in that way. Patch attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  7. Re: pgsql: Add function to log the memory contexts of specified backend pro

    torikoshia <torikoshia@oss.nttdata.com> — 2025-05-02T05:58:29Z

    On 2025-05-02 09:02, Fujii Masao wrote:
> On 2025/05/02 2:27, Fujii Masao wrote:
>> 
>> 
>> On 2025/05/01 21:42, Robert Haas wrote:
>>> On Thu, May 1, 2025 at 3:53 AM Fujii Masao 
>>> <masao.fujii@oss.nttdata.com> wrote:
>>>> 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:
>>> 
>>> I think that something like this could work, but you would need more
>>> than this. Otherwise, if the function errors out, the flag would
>>> remain permanently set.
>> 
>> Yes, we need to either use PG_TRY()/PG_FINALLY() or handle the flag as
>> a global variable and reset it in the error handling path. I think 
>> using
>> PG_TRY()/PG_FINALLY() is the simpler option.
> 
> I've implemented the patch in that way. Patch attached.

Thank you for the patch!

I confirmed that with this patch applied, the process no longer crashes 
even after applying the change Robert used to trigger the crash.

a small nitpick:

+                * requested  repeatedly and rapidly, potentially 
leading to infinite

It looks like there are two spaces between "requested" and "repeatedly".


-- 
Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.

  8. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-03T01:53:58Z

    
On 2025/05/02 14:58, torikoshia wrote:
> I confirmed that with this patch applied, the process no longer crashes even after applying the change Robert used to trigger the crash.
> 
> a small nitpick:
> 
> +                * requested  repeatedly and rapidly, potentially leading to infinite
> 
> It looks like there are two spaces between "requested" and "repeatedly".

Thanks for the review and testing! I've fixed the issue you pointed out.
Updated patch attached.

Since git cherry-pick didn't work cleanly for v16 and earlier,
I've also prepared a separate patch for those versions.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  9. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Robert Haas <robertmhaas@gmail.com> — 2025-05-05T14:57:52Z

    On Fri, May 2, 2025 at 9:54 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> Thanks for the review and testing! I've fixed the issue you pointed out.
> Updated patch attached.

Thanks for addressing this. However, I believe this commit may need to
take note of the following comment from elog.h:

 * Note: if a local variable of the function containing PG_TRY is modified
 * in the PG_TRY section and used in the PG_CATCH section, that variable
 * must be declared "volatile" for POSIX compliance.  This is not mere
 * pedantry; we have seen bugs from compilers improperly optimizing code
 * away when such a variable was not marked.  Beware that gcc's -Wclobbered
 * warnings are just about entirely useless for catching such oversights.

Based on this comment, I believe in_progress must be declared volatile.

As a stylistic comment, I think I would prefer making in_progress a
file-level global and giving it a less generic name (e.g.
LogMemoryContextInProgress). However, perhaps others will disagree.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  10. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-07T09:06:22Z

    
On 2025/05/05 23:57, Robert Haas wrote:
> On Fri, May 2, 2025 at 9:54 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> Thanks for the review and testing! I've fixed the issue you pointed out.
>> Updated patch attached.
> 
> Thanks for addressing this. However, I believe this commit may need to
> take note of the following comment from elog.h:

Thanks for the review!


>   * Note: if a local variable of the function containing PG_TRY is modified
>   * in the PG_TRY section and used in the PG_CATCH section, that variable
>   * must be declared "volatile" for POSIX compliance.  This is not mere
>   * pedantry; we have seen bugs from compilers improperly optimizing code
>   * away when such a variable was not marked.  Beware that gcc's -Wclobbered
>   * warnings are just about entirely useless for catching such oversights.
> 
> Based on this comment, I believe in_progress must be declared volatile.

You're right. OTOH, setting the flag inside the PG_TRY() block isn't necessary,
so I've moved it outside instead of leaving it inside and marking the flag volatile.


> As a stylistic comment, I think I would prefer making in_progress a
> file-level global and giving it a less generic name (e.g.
> LogMemoryContextInProgress). However, perhaps others will disagree.

I'm fine with this. I've renamed the flag and made it a file-level global
variable as suggested. Updated patch is attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  11. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-07-14T13:53:38Z

    
On 2025/05/07 18:06, Fujii Masao wrote:
> 
> 
> On 2025/05/05 23:57, Robert Haas wrote:
>> On Fri, May 2, 2025 at 9:54 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>> Thanks for the review and testing! I've fixed the issue you pointed out.
>>> Updated patch attached.
>>
>> Thanks for addressing this. However, I believe this commit may need to
>> take note of the following comment from elog.h:
> 
> Thanks for the review!
> 
> 
>>   * Note: if a local variable of the function containing PG_TRY is modified
>>   * in the PG_TRY section and used in the PG_CATCH section, that variable
>>   * must be declared "volatile" for POSIX compliance.  This is not mere
>>   * pedantry; we have seen bugs from compilers improperly optimizing code
>>   * away when such a variable was not marked.  Beware that gcc's -Wclobbered
>>   * warnings are just about entirely useless for catching such oversights.
>>
>> Based on this comment, I believe in_progress must be declared volatile.
> 
> You're right. OTOH, setting the flag inside the PG_TRY() block isn't necessary,
> so I've moved it outside instead of leaving it inside and marking the flag volatile.
> 
> 
>> As a stylistic comment, I think I would prefer making in_progress a
>> file-level global and giving it a less generic name (e.g.
>> LogMemoryContextInProgress). However, perhaps others will disagree.
> 
> I'm fine with this. I've renamed the flag and made it a file-level global
> variable as suggested. Updated patch is attached.

I've attached the rebased versions of the patches.

The patch for v14–v16 is labeled with a .txt extension to prevent cfbot
from treating it as a patch for master, which would cause it to fail
when applying.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  12. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Artem Gavrilov <artem.gavrilov@percona.com> — 2025-12-08T19:32:35Z

    Hi Fujii,

I did your patch review. It successfully applies to all targeted branches:
REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE
(4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and
master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every
revision. Everything seems fine with the patch, I think it's ready for
committer.

-- 

Artem Gavrilov

Senior Software Engineer, Percona

artem.gavrilov@percona.com
percona.com <http://www.percona.com>

  13. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Robert Haas <robertmhaas@gmail.com> — 2025-12-15T15:07:21Z

    On Mon, Dec 8, 2025 at 2:32 PM Artem Gavrilov
<artem.gavrilov@percona.com> wrote:
> I did your patch review. It successfully applies to all targeted branches: REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE (4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every revision. Everything seems fine with the patch, I think it's ready for committer.

LGTM, too.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  14. Re: pgsql: Add function to log the memory contexts of specified backend pro

    Fujii Masao <masao.fujii@gmail.com> — 2025-12-19T03:17:01Z

    On Tue, Dec 16, 2025 at 12:07 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Dec 8, 2025 at 2:32 PM Artem Gavrilov
> <artem.gavrilov@percona.com> wrote:
> > I did your patch review. It successfully applies to all targeted branches: REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE (4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every revision. Everything seems fine with the patch, I think it's ready for committer.
>
> LGTM, too.

Thanks to both of you for the review! I've pushed the patch.

Regards,

-- 
Fujii Masao