Re: Improve logging when using Huge Pages

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, sfrost(at)snowman(dot)net, alvherre(at)alvh(dot)no-ip(dot)org, andres(at)anarazel(dot)de, nathandbossart(at)gmail(dot)com, jchampion(at)timescale(dot)com, john(dot)naylor(at)enterprisedb(dot)com, masao(dot)fujii(at)oss(dot)nttdata(dot)com, noriyoshi(dot)shinoda(at)hpe(dot)com, pgsql-hackers(at)postgresql(dot)org, rjuju123(at)gmail(dot)com, sawada(dot)mshk(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, thomas(dot)munro(at)gmail(dot)com
Subject: Re: Improve logging when using Huge Pages
Date: 2023-03-24 01:50:50
Message-ID: ZB0B+u5RMN8g52kF@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 23, 2023 at 05:25:46PM +0900, Kyotaro Horiguchi wrote:
> Wouldn't storing the value in the shared memory itself work? Though,
> that space does become almost dead for the server's lifetime...

I'm sure it's possible, but it's also not worth writing a special
implementation just to handle huge_pages_active, which is better written
in 30 lines than in 300 lines.

If we needed to avoid using a GUC, maybe it'd work to add
huge_pages_active to PGShmemHeader. But "avoid using gucs at all costs"
isn't the goal, and using a GUC parallels all the similar, and related
things without needing to allocate extra bits of shared_memory.

On Thu, Mar 23, 2023 at 07:23:28AM +0900, Michael Paquier wrote:
> On Wed, Mar 22, 2023 at 05:18:28PM -0500, Justin Pryzby wrote:
> > Wow, good point. I think to make it work we'd need put
> > huge_pages_active into BackendParameters and handle it in
> > save_backend_variables(). If so, that'd be strong argument for using a
> > GUC, which already has all the necessary infrastructure for exposing the
> > server's state.
>
> I am afraid so, duplicating an existing infrastructure for a need like
> the one of this thread is not really appealing.

This goes back to using a GUC, and:

- removes GUC_RUNTIME_COMPUTED, since that causes a useless error when
using -C if the server is running, rather than successfully printing
"unknown".
- I also renamed it from huge_pages_active = {true,false,unknown} to
huge_pages_STATUS = {on,off,unknown}. This parallels huge_pages,
which is documented to accept values on/off/try. Also, true/false
isn't how bools are displayed.
- I realized that the rename suggested implementing it as an enum GUC,
and re-using the existing HUGE_PAGES_{ON,OFF} values (and adding an
"UNKNOWN"). Maybe this also avoids Stephen's earlier objection to
using a string ?

I mis-used cirrusci to check that the GUC does work correctly under
windows.

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.

--
Justin

Attachment Content-Type Size
v6-0001-add-GUC-huge_pages_active.patch text/x-diff 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2023-03-24 01:59:57 Re: Commitfest 2023-03 starting tomorrow!
Previous Message Peter Smith 2023-03-24 01:48:47 Re: Data is copied twice when specifying both child and parent table in publication