Release postmaster working memory context in slotsync worker

Started by Fujii Masao3 months ago7 messageshackers
Jump to latest
#1Fujii Masao
masao.fujii@gmail.com

Hi,

Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

To avoid this unnecessary memory usage, I'd like to propose that the slotsync
worker release the postmaster's working memory context at startup.
A patch is attached.

Currently, pg_log_backend_memory_contexts() reports the following
postmaster-related memory contexts in the slotsync worker:

LOG: level: 2; Postmaster: 21984 total in 2 blocks; 5600 free (7
chunks); 16384 used
LOG: level: 3; ident parser context: 1024 total in 1 blocks; 784
free (0 chunks); 240 used
LOG: level: 3; hba parser context: 25600 total in 6 blocks; 9864
free (11 chunks); 15736 used

With the attached patch, these contexts are no longer present.

Regards,

--
Fujii Masao

Attachments:

v1-0001-Release-postmaster-working-memory-context-in-slot.patchapplication/octet-stream; name=v1-0001-Release-postmaster-working-memory-context-in-slot.patchDownload+7-1
#2Andres Freund
andres@anarazel.de
In reply to: Fujii Masao (#1)
Re: Release postmaster working memory context in slotsync worker

Hi,

On 2026-02-28 01:25:12 +0900, Fujii Masao wrote:

Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

To avoid this unnecessary memory usage, I'd like to propose that the slotsync
worker release the postmaster's working memory context at startup.
A patch is attached.

Obviously this inconsistency is not good. However:

I think we should consider *not* releasing postmaster memory. Freeing the
memory actually can lead to an *increase* in memory usage and a slight
*decrease* in connection startup performance. The reason for that is that with
fork, memory allocated in postmaster is handled by copy-on-write in the
children.

If the memory allocated by postmaster is freed in the children, the process of
freeing requires some modifications to copy-on-write memory (leading to an
increase in memory usage and page faults) and it allows the system allocator
to reuse the malloc'd regions, which then leads to more copy on write.

It's however annoying that the memory shows up in pg_backend_memory_contexts,
so maybe what postmaster children should do is to instead is to move its
parent to be NULL? That will still incur COW copying (due to modifying
PostmasterContext's ->{parent,prevchild,nextchild} pointers), but only a
single page, instead of multiple pages.

Or perhaps PostmasterContext should just not be below TopMemoryContext, then
we wouldn't need to do anything.

The tradeoff might be different if postmaster modified its allocated memory
more, since that also leads to CoW, but from what I can tell, that doesn't
happen very much.

See [1]/messages/by-id/hgs2vs74tzxigf2xqosez7rpf3ia5e7izalg5gz3lv3nqfptxx@thanmprbpl4el for some numbers.

Perhaps this is not worth worrying about - currently with openssl linked in,
the bulk of the overhead is from that. But as the numbers in [1]/messages/by-id/hgs2vs74tzxigf2xqosez7rpf3ia5e7izalg5gz3lv3nqfptxx@thanmprbpl4el show, the
MemoryContextDelete(PostmasterContext) does make a difference.

Greetings,

Andres Freund

[1]: /messages/by-id/hgs2vs74tzxigf2xqosez7rpf3ia5e7izalg5gz3lv3nqfptxx@thanmprbpl4el

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: Release postmaster working memory context in slotsync worker

Andres Freund <andres@anarazel.de> writes:

On 2026-02-28 01:25:12 +0900, Fujii Masao wrote:

Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

Obviously this inconsistency is not good. However:
I think we should consider *not* releasing postmaster memory. Freeing the
memory actually can lead to an *increase* in memory usage and a slight
*decrease* in connection startup performance. The reason for that is that with
fork, memory allocated in postmaster is handled by copy-on-write in the
children.

Meh. I think that's optimizing for the wrong thing. To my mind the
point of releasing that context is to be sure that child processes
don't have access to postmaster-private data. Admittedly, we're not
doing anything as drastic as zeroing out the memory, but it'll soon
be overwritten as the child starts up and populates its caches.

regards, tom lane

#4Fujii Masao
masao.fujii@gmail.com
In reply to: Tom Lane (#3)
Re: Release postmaster working memory context in slotsync worker

On Sat, Feb 28, 2026 at 2:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2026-02-28 01:25:12 +0900, Fujii Masao wrote:

Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

Obviously this inconsistency is not good. However:
I think we should consider *not* releasing postmaster memory. Freeing the
memory actually can lead to an *increase* in memory usage and a slight
*decrease* in connection startup performance. The reason for that is that with
fork, memory allocated in postmaster is handled by copy-on-write in the
children.

Meh. I think that's optimizing for the wrong thing. To my mind the
point of releasing that context is to be sure that child processes
don't have access to postmaster-private data.

Okay, I've included this point in the commit message of the patch.

Admittedly, we're not
doing anything as drastic as zeroing out the memory, but it'll soon
be overwritten as the child starts up and populates its caches.

Yes.

Attached is a rebased version of the patch. I'm thinking to commit it.

Regards,

--
Fujii Masao

Attachments:

v2-0001-Release-postmaster-working-memory-context-in-slot.patchapplication/octet-stream; name=v2-0001-Release-postmaster-working-memory-context-in-slot.patchDownload+7-1
#5getiancheng
getiancheng_2012@163.com
In reply to: Fujii Masao (#4)
Re: Release postmaster working memory context in slotsync worker

---- Replied Message ----
| From | Fujii Masao<masao.fujii@gmail.com> |
| Date | 3/18/2026 14:31 |
| To | Tom Lane<tgl@sss.pgh.pa.us> |
| Cc | Andres Freund<andres@anarazel.de>,
PostgreSQL Hackers<pgsql-hackers@lists.postgresql.org> |
| Subject | Re: Release postmaster working memory context in slotsync worker |
On Sat, Feb 28, 2026 at 2:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:
On 2026-02-28 01:25:12 +0900, Fujii Masao wrote:
Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

Obviously this inconsistency is not good. However:
I think we should consider *not* releasing postmaster memory. Freeing the
memory actually can lead to an *increase* in memory usage and a slight
*decrease* in connection startup performance. The reason for that is that with
fork, memory allocated in postmaster is handled by copy-on-write in the
children.

Meh. I think that's optimizing for the wrong thing. To my mind the
point of releasing that context is to be sure that child processes
don't have access to postmaster-private data.

Okay, I've included this point in the commit message of the patch.

Admittedly, we're not
doing anything as drastic as zeroing out the memory, but it'll soon
be overwritten as the child starts up and populates its caches.

Yes.

Attached is a rebased version of the patch. I'm thinking to commit it.

Regards,

--
Fujii Masao

Hi

The change LGTM, since other worker processes do the same thing.
I am a bit confused with the comment message "ensuring it does not have access to postmaster-private data".
As a child of postmaster, the worker inherits PostmasterContext at fork() time, so the data is already copied into the child's address space. MemoryContextDelete() does not wipe that memory, so I don't think this literally "ensures it does not have access" to the data.
My understanding is that the main purpose here is to make sure the child does not continue to use PostmasterContext by mistake, rather than to prevent access in a strict sense. Maybe the commit message could say that a bit more precisely.
Regards,
Tiancheng Ge

#6Chao Li
li.evan.chao@gmail.com
In reply to: getiancheng (#5)
Re: Release postmaster working memory context in slotsync worker

On Mar 18, 2026, at 21:41, getiancheng <getiancheng_2012@163.com> wrote:

---- Replied Message ----
From Fujii Masao<masao.fujii@gmail.com>
Date 3/18/2026 14:31
To Tom Lane<tgl@sss.pgh.pa.us>
Cc Andres Freund<andres@anarazel.de>,
PostgreSQL Hackers<pgsql-hackers@lists.postgresql.org>
Subject Re: Release postmaster working memory context in slotsync worker
On Sat, Feb 28, 2026 at 2:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:
On 2026-02-28 01:25:12 +0900, Fujii Masao wrote:
Child processes do not need the postmaster's working memory context and
release it at the start of their main function. However, the slotsync worker
appears to have missed this step.

Obviously this inconsistency is not good. However:
I think we should consider *not* releasing postmaster memory. Freeing the
memory actually can lead to an *increase* in memory usage and a slight
*decrease* in connection startup performance. The reason for that is that with
fork, memory allocated in postmaster is handled by copy-on-write in the
children.

Meh. I think that's optimizing for the wrong thing. To my mind the
point of releasing that context is to be sure that child processes
don't have access to postmaster-private data.

Okay, I've included this point in the commit message of the patch.

Admittedly, we're not
doing anything as drastic as zeroing out the memory, but it'll soon
be overwritten as the child starts up and populates its caches.

Yes.

Attached is a rebased version of the patch. I'm thinking to commit it.

Regards,

--
Fujii Masao

Hi

The change LGTM, since other worker processes do the same thing.

Also LGTM.

I am a bit confused with the comment message "ensuring it does not have access to postmaster-private data".
As a child of postmaster, the worker inherits PostmasterContext at fork() time, so the data is already copied into the child's address space. MemoryContextDelete() does not wipe that memory, so I don't think this literally "ensures it does not have access" to the data.
My understanding is that the main purpose here is to make sure the child does not continue to use PostmasterContext by mistake, rather than to prevent access in a strict sense. Maybe the commit message could say that a bit more precisely.

I had the same feeling when I first read that commit message. But after reading Andres' email, I think I understand why that wording was added.

Still, the current phrasing seems a bit too strong to me. Strictly speaking, the memory is already inherited at fork time, so this is not making access impossible in an absolute sense. What this patch really does is remove the inherited PostmasterContext, so the child no longer retains that postmaster-private data through that context.

Maybe we could phrase in some way like: "so that it no longer retains access to postmaster-private data through PostmasterContext”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

#7Fujii Masao
masao.fujii@gmail.com
In reply to: Chao Li (#6)
Re: Release postmaster working memory context in slotsync worker

On Thu, Mar 19, 2026 at 5:16 PM Chao Li <li.evan.chao@gmail.com> wrote:

Still, the current phrasing seems a bit too strong to me. Strictly speaking, the memory is already inherited at fork time, so this is not making access impossible in an absolute sense. What this patch really does is remove the inherited PostmasterContext, so the child no longer retains that postmaster-private data through that context.

Maybe we could phrase in some way like: "so that it no longer retains access to postmaster-private data through PostmasterContext”.

Thanks for the review! I've pushed the patch.
Per your and Tiancheng's suggestions, I just used "prevent unintended use"
in the commit message.

Regards,

--
Fujii Masao