Missing initialization steps in --check and --single modes - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Missing initialization steps in --check and --single modes |
Date | |
Msg-id | 2081982.1734393311@sss.pgh.pa.us Whole thread Raw |
Responses |
Re: Missing initialization steps in --check and --single modes
|
List | pgsql-hackers |
I was experimenting today with running initdb under low-resource situations (per nearby thread about OpenBSD), and I realized that "postgres --check" does not provide an adequate check on whether the specified number of semaphores can be created. That's because it fails to check whether we can still open a reasonable number of files after we've opened the semaphores, and on platforms where semaphores eat file descriptors, that matters. The lack of field complaints about this is probably because there are no common platforms on which we choose a semaphore implementation that consumes FDs. (I ran into it while checking whether modern NetBSD supports unnamed POSIX semaphores. Seems it does, but it uses an FD for each one, and that results in initdb overestimating what max_connections it can choose.) Nonetheless, this seems not totally academic, because the same code path is also used in --boot mode. In that mode, our failure to call set_max_safe_fds() will result in fd.c using a conservatively tiny limit on the number of FDs it can have open, which probably has some small penalty on the runtime of initdb. While comparing bootstrap.c to postmaster.c, I also noticed that bootstrap mode is failing to call set_stack_base(). That means that our checks for stack overflow are inoperative in bootstrap mode, which doesn't seem great. The same omissions appear in PostgresSingleUserMain, meaning that --single mode also operates with few FDs and no stack depth protection. That's considerably less than great. Hence I propose the attached. I'm leaning towards not back-patching given that these issues seem pretty minor ... but maybe for --single mode they're not so minor? regards, tom lane diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index a5217773ff..cffd63dcc0 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -334,6 +334,12 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) CreateSharedMemoryAndSemaphores(); + /* + * Estimate number of openable files. This is essential too in --check + * mode, because on some platforms semaphores count as open files. + */ + set_max_safe_fds(); + /* * XXX: It might make sense to move this into its own function at some * point. Right now it seems like it'd cause more code duplication than @@ -346,6 +352,11 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) abort(); } + /* + * Set reference point for stack-depth checking. + */ + (void) set_stack_base(); + /* * Do backend-like initialization for bootstrap mode */ diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index e0a603f42b..20554e0277 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4238,8 +4238,23 @@ PostgresSingleUserMain(int argc, char *argv[], */ InitializeWalConsistencyChecking(); + /* + * Create shared memory etc. (Nothing's really "shared" in single-user + * mode, but we must have these data structures anyway.) + */ CreateSharedMemoryAndSemaphores(); + /* + * Estimate number of openable files. This must happen after setting up + * semaphores, because on some platforms semaphores count as open files. + */ + set_max_safe_fds(); + + /* + * Set reference point for stack-depth checking. + */ + (void) set_stack_base(); + /* * Remember stand-alone backend startup time,roughly at the same point * during startup that postmaster does so.
pgsql-hackers by date: