Re: Improve logging when using Huge Pages

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Nathan Bossart <nathandbossart@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, thomas.munro@gmail.com, tgl@sss.pgh.pa.us, john.naylor@enterprisedb.com, noriyoshi.shinoda@hpe.com, jchampion@timescale.com, sawada.mshk@gmail.com, masao.fujii@oss.nttdata.com, pgsql-hackers@postgresql.org, rjuju123@gmail.com
Date: 2023-03-13T20:03:50Z
Lists: pgsql-hackers

Attachments

On Thu, Mar 09, 2023 at 03:02:29PM -0500, Stephen Frost wrote:
> * Justin Pryzby (pryzby@telsasoft.com) wrote:
> > On Thu, Mar 09, 2023 at 09:34:10AM -0500, Stephen Frost wrote:
> > > * Nathan Bossart (nathandbossart@gmail.com) wrote:
> > > > On Wed, Feb 15, 2023 at 10:13:17AM -0800, Nathan Bossart wrote:
> > > > > On Tue, Feb 14, 2023 at 07:32:56PM -0600, Justin Pryzby wrote:
> > > > >> On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote:
> > > > >>> I'm curious why you chose to make this a string instead of an enum.  There
> > > > >>> might be little practical difference, but since there are only three
> > > > >>> possible values, I wonder if it'd be better form to make it an enum.
> > > > >> 
> > > > >> It takes more code to write as an enum - see 002.txt.  I'm not convinced
> > > > >> this is better.
> > > > >> 
> > > > >> But your comment made me fix its <type>, and reconsider the strings,
> > > > >> which I changed to active={unknown/true/false} rather than {unk/on/off}.
> > > > >> It could also be active={unknown/yes/no}...
> > > > > 
> > > > > I think unknown/true/false is fine.  I'm okay with using a string if no one
> > > > > else thinks it should be an enum (or a bool).
> > > > 
> > > > There's been no response for this, so I guess we can proceed with a string
> > > > GUC.
> > > 
> > > Just happened to see this and I'm not really a fan of this being a
> > > string when it's pretty clear that's not what it actually is.
> > 
> > I don't understand what you mean by that.
> > Why do you say it isn't a string ?
> 
> Because it's clearly a yes/no, either the server is currently running
> with huge pages, or it isn't.  That's the definition of a boolean.

I originally implemented it as a boolean, and I changed it in response
to the feedback that postgres -C huge_pages_active should return
"unknown".

> > Is there an agreement to use a function, instead ?

Alvaro was -1 on using a function
Andres is +0 (?)
Nathan is +1
Stephen is +1

I'm -0.5, but I reimplemented it as a function.  I hope that helps it to
progress.  Please include a suggestion if there's better place for the
function or global var.

-- 
Justin

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add GUC parameter "huge_pages_status"

  2. Add check on initial and boot values when loading GUCs

  3. Clean up some inconsistencies with GUC declarations

  4. Clean up some GUC declarations and comments