public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Bossart, Nathan <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Mark Dilger <[email protected]>
Cc: Don Seiler <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Estimating HugePages Requirements?
Date: Wed, 8 Sep 2021 12:50:08 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On Tue, Sep 07, 2021 at 05:08:43PM +0000, Bossart, Nathan wrote:
> On 9/6/21, 9:00 PM, "Michael Paquier" <[email protected]> wrote:
>> + sprintf(buf, "%lu MB", size_mb);
>> + SetConfigOption("shared_memory_size", buf, PGC_INTERNAL, PGC_S_OVERRIDE);
>> One small-ish comment about 0002: there is no need to add the unit
>> into the buffer set as GUC_UNIT_MB would take care of that. The patch
>> looks fine.
>
> I fixed this in v7.
Switched the variable name to shared_memory_size_mb for easier
grepping, moved it to a more correct location with the other read-only
GUCS, and applied 0002. Well, 0001 here.
>> +#ifndef WIN32
>> +#include <sys/mman.h>
>> +#endif
>> So, this is needed in ipci.c to check for MAP_HUGETLB. I am not much
>> a fan of moving around platform-specific checks when these have
>> remained local to each shmem implementation. Could it be cleaner to
>> add GetHugePageSize() to win32_shmem.c and make it always declared in
>> the SysV implementation?
>
> I don't know if it's really all that much cleaner, but I did it this
> way in v7. IMO it's a little weird that GetHugePageSize() doesn't
> return the value from GetLargePageMinimum(), but that's what we'd need
> to do to avoid setting huge_pages_required for Windows without any
> platform-specific checks.
Thanks. Keeping MAP_HUGETLB within the SysV portions is an
improvement IMO. That's subject to one's taste, perhaps.
After sleeping on it, I'd be fine to live with the logic based on the
new GUC flag called GUC_RUNTIME_COMPUTED to control if a parameter can
be looked at either an earlier or a later stage of the startup
sequences, with the later stage meaning that such parameters cannot be
checked if a server is running. This case was originally broken
anyway, so it does not make it worse, just better.
+ This can be used on a running server for most parameters. However,
+ the server must be shut down for some runtime-computed parameters
+ (e.g., <xref linkend="guc-huge-pages-required"/>).
Perhaps we should add a couple of extra parameters here, like
shared_memory_size, and perhaps wal_segment_size? The list does not
have to be complete, just meaningful enough.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
view thread (108+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Estimating HugePages Requirements?
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox