Re: Improve logging when using Huge Pages
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, sfrost@snowman.net, alvherre@alvh.no-ip.org, andres@anarazel.de, jchampion@timescale.com, john.naylor@enterprisedb.com, masao.fujii@oss.nttdata.com, noriyoshi.shinoda@hpe.com, pgsql-hackers@postgresql.org, rjuju123@gmail.com, sawada.mshk@gmail.com, tgl@sss.pgh.pa.us, thomas.munro@gmail.com
Date: 2023-06-23T04:57:51Z
Lists: pgsql-hackers
Attachments
- v9-0001-add-GUC-huge_pages_status.patch (text/x-diff) patch v9-0001
On Tue, Jun 20, 2023 at 06:44:20PM -0500, Justin Pryzby wrote:
> On Tue, Jun 13, 2023 at 02:50:30PM +0900, Michael Paquier wrote:
>> On Mon, Jun 12, 2023 at 02:37:15PM -0700, Nathan Bossart wrote:
>> > Fair enough. I know I've been waffling in the GUC versus function
>> > discussion, but FWIW v7 of the patch looks reasonable to me.
>>
>> + Assert(strcmp("unknown", GetConfigOption("huge_pages_status", false, false)) != 0);
>>
>> Not sure that there is anything to gain with this assertion in
>> CreateSharedMemoryAndSemaphores(), because this is pretty much what
>> check_GUC_init() looks after?
>
> It seems like you misread the assertion, so I added a comment about it.
> Indeed, the assertion addresses the other question you asked later.
>
> That's what I already commented about, and the reason I found it
> compelling not to use a boolean.
Apologies for the late reply here.
At the end, I am on board with the addition of this assertion and its
position after PGSharedMemoryCreate().
I would also move the SetConfigOption() for the WIN32 path after ew
have passed all the checks. There are a few FATALs that can be
triggered so it would be a waste to call it if we are going to fail
the shmem creation in this path.
I could not resist adding two checks in the TAP tests to make sure
that we don't report unknown. Perhaps that's not necessary, but that
would provide coverage in a more broader way, and these are cheap.
I have run one indentation, while on it.
Note to self: check that manually on Windows.
--
Michael
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Add GUC parameter "huge_pages_status"
- a14354cac0e3 17.0 landed
-
Add check on initial and boot values when loading GUCs
- a73952b79563 16.0 cited
-
Clean up some inconsistencies with GUC declarations
- d9d873bac670 16.0 cited
-
Clean up some GUC declarations and comments
- 7d25958453a6 16.0 cited