On Fri, Nov 24, 2017 at 9:28 AM, Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com> wrote: > From: Thomas Munro [mailto:thomas.munro@enterprisedb.com] >> I hope Tsunakawa-san doesn't mind me posting another rebased version of >> his patch. The last version conflicted with the change from SGML to XML >> that just landed in commit 3c49c6fa. > > Thank you very much for keeping it fresh. I hope the committer could have some time.
I have moved this patch to next CF. Magnus, you are registered as a reviewer of this patch. Are you planning to look at it and potentially commit it?
Apologies for the delays here. Yes, I was and am.
I took a look at this patch again. I've made some small updates to comments etc. There was also from what I can tell one actual bug in the code -- a missing free() of delPrivs.
The debug message for ERROR_NO_SYSTEM_RESOURCES said it turned off huge pages because of not enough huge pages, but AIUI it can be because of other resources as well. So I dropped that specific.
However, reading through a few things -- we now use the API GetLargePageMinimum() unconditionally. This API appeared in Windows Vista and Windows 2003. That means that if we apply this patch, those are now our minimum versions of Windows. At least unless Microsoft backpatched things.
This is probably equally true of some other things we do, but those are runtime, and we would just give an error on old platforms. If I'm thinking right, then direct linking to GetLargePageMinimum() will simply make it impossible to start a PostgreSQL backend with or without huge pages on previous versions, because it will give a linker error.
I wonder if we need to do something similar to what we already do for CreateRestrictedToken() in pg_ctl.c for example. That one actually is done for compatibility with NT4/2000 -- CreateRestrictedToken was added in XP. So while we could consider getting rid of that workaround now, perhaps we need to put a similar one in place for this?
I don't have a Windows build box around ATM, or a Windows XP, so if somebody could try the attached version, build a postgres and see if it even starts on a Windows XP machine, that would be useful input!
If we can confirm/deny the situation around XP and decide what to do about it, I am now happy to commit the rest of this patch.
