Re: Improve logging when using Huge Pages

On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote:
> On Mon, Feb 13, 2023 at 05:22:45PM -0600, Justin Pryzby wrote:
> > +        Reports whether huge pages are in use by the current process.
> > +        See <xref linkend="guc-huge-pages"/> for more information.
> 
> nitpick: Should this say "server" instead of "current process"?

It should probably say "instance" :)

> > +static char *huge_pages_active = "unknown"; /* dynamically set */
> 
> nitpick: Does this need to be initialized here?

None of the GUCs' C vars need to be initialized, since the guc machinery
will do it. 

...but the convention is that they *are* initialized - and that's now
partially enforced.

See:
d9d873bac67047cfacc9f5ef96ee488f2cb0f1c3
7d25958453a60337bcb7bcc986e270792c007ea4
a73952b795632b2cf5acada8476e7cf75857e9be

> > +    {
> > +        {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
> > +            gettext_noop("Indicates whether huge pages are in use."),
> > +            NULL,
> > +            GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
> > +        },
> > +        &huge_pages_active,
> > +        "unknown",
> > +        NULL, NULL, NULL
> > +    },
> 
> 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}...

-- 
Justin