Horton

Thread

Commits

  1. Detach syslogger from shared memory

  2. Refactor postmaster child process launching

  1. Detach shared memory in Postmaster child if not needed

    Rui Zhao <xiyuan.zr@alibaba-inc.com> — 2024-07-29T09:57:06Z

    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

  2. Re: Detach shared memory in Postmaster child if not needed

    Aleksander Alekseev <aleksander@timescale.com> — 2024-07-29T11:44:20Z

    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

  3. Re：Detach shared memory in Postmaster child if not needed

    Rui Zhao <xiyuan.zr@alibaba-inc.com> — 2024-07-29T17:25:02Z

    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

  4. Re: Detach shared memory in Postmaster child if not needed

    Robert Haas <robertmhaas@gmail.com> — 2024-07-29T18:10:27Z

    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

  5. Re: Detach shared memory in Postmaster child if not needed

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-07-29T19:41:44Z

    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)