Detach shared memory in Postmaster child if not needed
Prior to PG16, postmaster children would manually detach from shared memory
if it was not needed. However, this behavior was removed in fork mode in
commit aafc05d.
Detaching shared memory when it is no longer needed is beneficial, as
postmaster children (like syslogger) don't wish to take any risk of
accidentally corrupting shared memory. Additionally, any panic in these
processes will not reset shared memory.
The attached patch addresses this issue by detaching shared memory after
fork_process().
Best regard,
Rui Zhao
Attachments:
0001-Detach-shared-memory-in-Postmaster-child-if-not-needed.patchapplication/octet-streamDownload
From 1b1c07f6681bf261f1c19ad8e7ad84f008a1adea Mon Sep 17 00:00:00 2001
From: "Rui Zhao" <xiyuan.zr@alibaba-inc.com>
Date: Mon, 29 Jul 2024 16:55:04 +0800
Subject: [PATCH] Detach shared memory in Postmaster child if not needed
---
src/backend/postmaster/launch_backend.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index fafe5feecc..9b018ef500 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -249,6 +249,13 @@ postmaster_child_launch(BackendType child_type,
/* Detangle from postmaster */
InitPostmasterChild();
+ /* Detach shared memory if not needed. */
+ if (!child_process_kinds[child_type].shmem_attach)
+ {
+ dsm_detach_all();
+ PGSharedMemoryDetach();
+ }
+
/*
* Enter the Main function with TopMemoryContext. The startup data is
* allocated in PostmasterContext, so we cannot release it here yet.
--
2.39.3
Hi Rui,
Prior to PG16, postmaster children would manually detach from shared memory
if it was not needed. However, this behavior was removed in fork mode in
commit aafc05d.Detaching shared memory when it is no longer needed is beneficial, as
postmaster children (like syslogger) don't wish to take any risk of
accidentally corrupting shared memory. Additionally, any panic in these
processes will not reset shared memory.The attached patch addresses this issue by detaching shared memory after
fork_process().
Thanks for the patch. How do you estimate its performance impact?
Note the comments for postmaster_child_launch(). This function is
exposed to the third-party code and guarantees to attach shared
memory. I doubt that there is much third-party code in existence to
break but you should change to comment.
--
Best regards,
Aleksander Alekseev
Thanks for your reply.
Thanks for the patch. How do you estimate its performance impact?
In my patch, ony child processes that set
(child_process_kinds[child_type].shmem_attach == false)
will detach from shared memory.
Child processes with B_STANDALONE_BACKEND and B_INVALID don't call
postmaster_child_launch().
Therefore, currently, only the syslogger will be affected,
which should be harmless.
Note the comments for postmaster_child_launch(). This function is
exposed to the third-party code and guarantees to attach shared
memory. I doubt that there is much third-party code in existence to
break but you should change to comment.
Thank you for your reminder. My v2 patch will include the comments for
postmaster_child_launch().
--
Best regards,
Rui Zhao
Attachments:
0001-Detach-shared-memory-in-Postmaster-child-if-not-needed-v2.patchapplication/octet-streamDownload
From 96f869371e77e21047e1b842692872d3ebf445de Mon Sep 17 00:00:00 2001
From: "Rui Zhao" <xiyuan.zr@alibaba-inc.com>
Date: Mon, 29 Jul 2024 16:55:04 +0800
Subject: [PATCH] Detach shared memory in Postmaster child if not needed
---
src/backend/postmaster/launch_backend.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index 8d4589846a..0037772c4f 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -219,7 +219,8 @@ PostmasterChildName(BackendType child_type)
* Start a new postmaster child process.
*
* The child process will be restored to roughly the same state whether
- * EXEC_BACKEND is used or not: it will be attached to shared memory, and fds
+ * EXEC_BACKEND is used or not: it will be attached to shared memory if
+ * shmem_attach is set to true and will be detached if it is not, and fds
* and other resources that we've inherited from postmaster that are not
* needed in a child process have been closed.
*
@@ -249,6 +250,13 @@ postmaster_child_launch(BackendType child_type,
/* Detangle from postmaster */
InitPostmasterChild();
+ /* Detach from shared memory if it is not needed. */
+ if (!child_process_kinds[child_type].shmem_attach)
+ {
+ dsm_detach_all();
+ PGSharedMemoryDetach();
+ }
+
/*
* Enter the Main function with TopMemoryContext. The startup data is
* allocated in PostmasterContext, so we cannot release it here yet.
--
2.39.3
On Mon, Jul 29, 2024 at 5:57 AM Rui Zhao <xiyuan.zr@alibaba-inc.com> wrote:
Prior to PG16, postmaster children would manually detach from shared memory
if it was not needed. However, this behavior was removed in fork mode in
commit aafc05d.
Oh. The commit message makes no mention of that. I wonder whether it
was inadvertent.
Detaching shared memory when it is no longer needed is beneficial, as
postmaster children (like syslogger) don't wish to take any risk of
accidentally corrupting shared memory. Additionally, any panic in these
processes will not reset shared memory.
+1.
The attached patch addresses this issue by detaching shared memory after
fork_process().
I don't know whether this is the correct approach or not, but
hopefully Heikki can comment.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 29/07/2024 21:10, Robert Haas wrote:
On Mon, Jul 29, 2024 at 5:57 AM Rui Zhao <xiyuan.zr@alibaba-inc.com> wrote:
Prior to PG16, postmaster children would manually detach from shared memory
if it was not needed. However, this behavior was removed in fork mode in
commit aafc05d.Oh. The commit message makes no mention of that. I wonder whether it
was inadvertent.Detaching shared memory when it is no longer needed is beneficial, as
postmaster children (like syslogger) don't wish to take any risk of
accidentally corrupting shared memory. Additionally, any panic in these
processes will not reset shared memory.+1.
The attached patch addresses this issue by detaching shared memory after
fork_process().I don't know whether this is the correct approach or not, but
hopefully Heikki can comment.
Good catch, it was not intentional. The patch looks good to me, so
committed. Thanks Rui!
--
Heikki Linnakangas
Neon (https://neon.tech)