Subject: Re: Improve logging when using Huge Pages

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Mar 2023 14:17:33 +0900

On Mon, Mar 20, 2023 at 01:09:09AM -0400, Tom Lane wrote:
> I'm confused here, because Horiguchi-san is saying that that
> won't work.  I've not checked the code lately, but I think that
> "postgres -C var" prints its results before actually attempting
> to establish shared memory, so I suspect Horiguchi-san is right.

Yes, I haven't read correctly through.  Sorry for the noise.
--
Michael

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 21 Mar 2023 21:19:41 -0500

On Mon, Mar 20, 2023 at 02:03:13PM +0900, Michael Paquier wrote:
> On Mon, Mar 20, 2023 at 01:54:46PM +0900, Michael Paquier wrote:
> > The main advantage of a read-only GUC over a function is that users
> > would not need to start a postmaster to know if huge pages would be
> > active or not.  This is the main reason why a GUC would be a better
> > fit, in my opinion, because it makes for a cheaper check, while still
> > allowing a SQL query to check the value of the GUC.
> 
> [ Should have read more carefully ]
> 
> ..  Which is something you cannot do with -C because mmap() happens
> after the runtime-computed logic for postgres -C.  It does not sound
> right to do the mmap() for a GUC check, so indeed a function may be
> more adapted rather than move mmap() call a bit earlier in the
> postmaster startup sequence.

> Yes, I haven't read correctly through.  Sorry for the noise.

You set this patch to "waiting on author" twice.  Would you let me know
what I could do to help progress the patch?  Right now, I have no idea.

Most recently, you said it'd be better implemented as a GUC to allow
using -C, but then recanted because -C doesn't work for this (which is
why I implemented it as a string back on 2023-02-08).  Which is why I
reset its status on 2023-03-20.

2023-03-22 01:36:58 	Michael Paquier (michael-kun) 	New status: Waiting on Author
2023-03-20 13:05:32 	Justin Pryzby (justinpryzby) 	New status: Needs review
2023-03-20 05:03:53 	Michael Paquier (michael-kun) 	New status: Waiting on Author

-- 
Justin

---

From: Michael Paquier <[email-masked]>
Date: Wed, 22 Mar 2023 16:37:01 +0900

> what I could do to help progress the patch?  Right now, I have no idea.

My mistake, I've been looking at an incorrect version of the patch.
Thanks for correcting me here.

I have read through the proposed v5 of the patch, that seems to be the
latest one available:
https://www.postgresql.org/message-id/ZA+Bpk/[email-masked]

I have noted something..  For the WIN32 case, we have that:

+++ b/src/backend/port/win32_shmem.c
@@ -327,6 +327,8 @@ retry:
            Sleep(1000);
            continue;
        }
+
+       huge_pages_active = ((flProtect & SEC_LARGE_PAGES) != 0);
        break;

Are you sure that this is correct?  This is set in
PGSharedMemoryCreate(), part of CreateSharedMemoryAndSemaphores() in
the startup sequence that creates the shmem segment.  However, for a
normal backend created by EXEC_BACKEND, SubPostmasterMain() reattaches
to an existing shared memory segment, so we don't go through the
creation path that would set huge_pages_active for the process just
started, (note that InitPostmasterChild() switches IsUnderPostmaster,
bypassing the shmem segment creation).
--
Michael