Re: Improve logging when using Huge Pages

On Tue, Mar 28, 2023 at 09:35:30AM +0900, Michael Paquier wrote:
> On Thu, Mar 23, 2023 at 08:50:50PM -0500, Justin Pryzby wrote:
>
> You mean that you abused of it in some custom branch running the CI on
> github? If I may ask, what did you do to make sure that huge pages
> are set when re-attaching a backend to a shmem area?

Yes. I hijacked a tap test to first run "show huge_pages_active" and then
also caused it to fail, such that I could check its output.

https://cirrus-ci.com/task/6309510885670912
https://cirrus-ci.com/task/6613427737591808

> > If there's continuing aversions to using a GUC, please say so, and try
> > to suggest an alternate implementation you think would be justified.
> > It'd need to work under windows with EXEC_BACKEND.
>
> Looking at the patch, it seems like that this should work even for
> EXEC_BACKEND on *nix when a backend reattaches.. It may be better to
> be sure of that, as well, if it has not been tested.

Arg, no - it wasn't working. I added an assert to check that a running
server won't output "unknown".

> +++ b/src/backend/port/win32_shmem.c
> @@ -327,6 +327,10 @@ retry:
> Sleep(1000);
> continue;
> }
> +
> + SetConfigOption("huge_pages_status", (flProtect & SEC_LARGE_PAGES) ?
> + "on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT)
>
> Perhaps better to just move that at the end of PGSharedMemoryCreate()
> for WIN32?

Maybe - I don't know.

> + <varlistentry id="guc-huge-pages-status" xreflabel="huge_pages_status">
> + <term><varname>huge_pages_status</varname> (<type>enum</type>)
> + <indexterm>
> + <primary><varname>huge_pages_status</varname> configuration parameter</primary>
> + </indexterm>
> + </term>
> + <listitem>
> + <para>
> + Reports the state of huge pages in the current instance.
> + See <xref linkend="guc-huge-pages"/> for more information.
> + </para>
> + </listitem>
> + </varlistentry>
>
> The patch needs to provide more details about the unknown state, IMO,
> to make it crystal-clear to the users what are the limitations of this
> GUC, especially regarding the fact that this is useful when "try"-ing
> to allocate huge pages, and that the value can only be consulted after
> the server has been started.

I'm not sure I agree. It can be confusing (even harmful) to overspecify the
documentation. I used the word "instance" specifically to refer to a running
server. Anyone querying the status of huge pages is going to need to
understand that it doesn't make sense to ask about the memory state of a server
that's not running. Actually, I'm having trouble imagining that anybody would
ever run postgres -C huge_page_status except if they wondered whether it might
misbehave. If what I've written is inadequate, could you propose something ?

--
Justin

PS. I hadn't updated this thread because my last message from you (on
another thread) indicated that you'd gotten busy. I'm hoping you'll
respond to these other inquiries when you're able.

https://www.postgresql.org/message-id/ZCUZLiCeXq4Im7OG%40telsasoft.com
https://www.postgresql.org/message-id/ZCUKL22GutwGrrZk%40telsasoft.com