Re: Fix early elog(FATAL)
Noah Misch <noah@leadboat.com>
From: Noah Misch <noah@leadboat.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: pgsql-hackers@postgresql.org
Date: 2024-12-14T03:15:05Z
Lists: pgsql-hackers
On Thu, Dec 12, 2024 at 10:07:00AM -0600, Nathan Bossart wrote: > On Wed, Dec 11, 2024 at 07:34:14PM -0800, Noah Misch wrote: > > On Tue, Dec 10, 2024 at 04:18:19PM -0600, Nathan Bossart wrote: > >> FWIW I'd probably vote for option 1. That keeps the initialization of the > >> globals together, reduces the call sites, and fixes the bug. I'd worry a > >> little about moving the MyProcPid assignments out of that function without > >> adding a bunch of commentary to explain why. > > > > Can you say more about that? A comment about MyProcPid could say "fork() is > > the one thing that changes the getpid() return value". To me, the things > > InitProcessGlobals() sets are all different. MyProcPid can be set without > > elog(ERROR) and gets invalidated at fork(). The others reasonably could > > elog(ERROR). (They currently don't.) The random state could have a different > > lifecycle. If we had a builtin pooler that reused processes, we'd > > reinitialize random state at each process reuse, not at each fork(). So I see > > the grouping of (MyProcPid, MyStartTimestamp, random seed) as mostly an > > accident of history. > > Fair enough. I suppose part of my hesitation stems from expecting hackers > to be more likely to remember to call InitProcessGlobals() than to > initialize MyProcPid. But given your change requires initializing > MyProcPid in exactly 2 places, and there are unlikely to be more in the > near future, I might be overthinking it. I don't feel strongly either way. I did write it the option-1 way originally. Then I started thinking about changes at a distance causing the other InitProcessGlobals() tasks to palloc or elog. We could do option-1 in master and keep the back branches in their current state.
Commits
-
Fix elog(FATAL) before PostmasterMain() or just after fork().
- ac4a2b40392b 16.7 landed
- 839da50bd431 15.11 landed
- 6151769f6ef5 13.19 landed
- 4bd9de3f4190 17.3 landed
- 315264d70128 14.16 landed
- 8b9cbf492275 18.0 landed
-
Remove obsolete check in SIGTERM handler for the startup process.
- 8fd0498de200 17.0 cited
-
Check that MyProcPid == getpid() in backend signal handlers.
- 3b00fdba9f20 17.0 cited
-
Avoid calling proc_exit() in processes forked by system().
- 97550c071197 17.0 cited
-
Refactor pid, random seed and start time initialization.
- 197e4af9d5da 12.0 cited