Heikki Linnakangas wrote: > On 27.12.2012 22:46, Alvaro Herrera wrote: > >Heikki Linnakangas wrote: > > > >>Might be cleaner to directly assign the correct value to MaxBackends > >>above, ie. "MaxBackends = MaxConnections + newval + 1 + > >>GetNumShmemAttachedBgworkers()". With a comment to remind that it > >>needs to be kept in sync with the other places where that > >>calculation is done, in guc.c. Or put that calculation in a new > >>function and call it above and in guc.c. > >> > >>Thinking about this some more, it might be cleaner to move the > >>responsibility of setting MaxBackends out of guc.c, into > >>postmaster.c. The guc machinery would set max_connections and > >>autovacuum_max_workers as usual, but not try to set MaxBackends. > >>After reading the config file in postmaster.c, calculate > >>MaxBackends.
Actually this patch still needed one more change, because we weren't rechecking that we're not beyond the MAX_BACKENDS value after bgworker registration. This is hard to hit, because with the current compile constants you need over eight million backends in total to hit that limit (and 360 GB of shared memory), but it seems dangerous to leave that without any protection. I kinda hate this a bit because I had to move MAX_BACKENDS from a private value in guc.c to postmaster.h. But the wording of the error message is what took the most time: + ereport(LOG, + (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED), + errmsg("too many background workers"), + errdetail("Up to %d background workers can be registered with the current settings.", + MAX_BACKENDS - (MaxConnections + autovacuum_max_workers + 1)))); Completely different ideas for this error message are welcome. My first try enumerated all the variables involved. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *************** *** 499,504 **** typedef struct --- 499,505 ---- bool redirection_done; bool IsBinaryUpgrade; int max_safe_fds; + int MaxBackends; #ifdef WIN32 HANDLE PostmasterHandle; HANDLE initial_signal_pipe; *************** *** 897,911 **** PostmasterMain(int argc, char *argv[]) process_shared_preload_libraries(); /* ! * If loadable modules have added background workers, MaxBackends needs to ! * be updated. Do so now by forcing a no-op update of max_connections. ! * XXX This is a pretty ugly way to do it, but it doesn't seem worth ! * introducing a new entry point in guc.c to do it in a cleaner fashion. */ ! if (GetNumShmemAttachedBgworkers() > 0) ! SetConfigOption("max_connections", ! GetConfigOption("max_connections", false, false), ! PGC_POSTMASTER, PGC_S_OVERRIDE); /* * Establish input sockets. --- 898,908 ---- process_shared_preload_libraries(); /* ! * Now that loadable modules have had their chance to register background ! * workers, calculate MaxBackends. Add one for the autovacuum launcher. */ ! MaxBackends = MaxConnections + autovacuum_max_workers + 1 + ! GetNumShmemAttachedBgworkers(); /* * Establish input sockets. *************** *** 5214,5219 **** RegisterBackgroundWorker(BackgroundWorker *worker) --- 5211,5227 ---- return; } + if (MaxConnections + autovacuum_max_workers + 1 + GetNumShmemAttachedBgworkers() >= + MAX_BACKENDS) + { + ereport(LOG, + (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED), + errmsg("too many background workers"), + errdetail("Up to %d background workers can be registered with the current settings.", + MAX_BACKENDS - (MaxConnections + autovacuum_max_workers + 1)))); + return; + } + /* * Copy the registration data into the registered workers list. */ *************** *** 5836,5841 **** save_backend_variables(BackendParameters *param, Port *port, --- 5844,5851 ---- param->IsBinaryUpgrade = IsBinaryUpgrade; param->max_safe_fds = max_safe_fds; + param->MaxBackends = MaxBackends; + #ifdef WIN32 param->PostmasterHandle = PostmasterHandle; if (!write_duplicated_handle(¶m->initial_signal_pipe, *************** *** 6061,6066 **** restore_backend_variables(BackendParameters *param, Port *port) --- 6071,6078 ---- IsBinaryUpgrade = param->IsBinaryUpgrade; max_safe_fds = param->max_safe_fds; + MaxBackends = param->MaxBackends; + #ifdef WIN32 PostmasterHandle = param->PostmasterHandle; pgwin32_initial_signal_pipe = param->initial_signal_pipe; *** a/src/backend/utils/init/globals.c --- b/src/backend/utils/init/globals.c *************** *** 103,115 **** int work_mem = 1024; int maintenance_work_mem = 16384; /* ! * Primary determinants of sizes of shared-memory structures. MaxBackends is ! * MaxConnections + autovacuum_max_workers + 1 (it is computed by the GUC ! * assign hooks for those variables): */ int NBuffers = 1000; - int MaxBackends = 100; int MaxConnections = 90; int VacuumCostPageHit = 1; /* GUC parameters for vacuum */ int VacuumCostPageMiss = 10; --- 103,116 ---- int maintenance_work_mem = 16384; /* ! * Primary determinants of sizes of shared-memory structures. ! * ! * MaxBackends is computed by PostmasterMain after modules have had a chance to ! * register background workers. */ int NBuffers = 1000; int MaxConnections = 90; + int MaxBackends = 0; int VacuumCostPageHit = 1; /* GUC parameters for vacuum */ int VacuumCostPageMiss = 10; *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** *** 103,119 **** #define MAX_KILOBYTES (INT_MAX / 1024) #endif - /* - * Note: MAX_BACKENDS is limited to 2^23-1 because inval.c stores the - * backend ID as a 3-byte signed integer. Even if that limitation were - * removed, we still could not exceed INT_MAX/4 because some places compute - * 4*MaxBackends without any overflow check. This is rechecked in - * check_maxconnections, since MaxBackends is computed as MaxConnections - * plus the number of bgworkers plus autovacuum_max_workers plus one (for the - * autovacuum launcher). - */ - #define MAX_BACKENDS 0x7fffff - #define KB_PER_MB (1024) #define KB_PER_GB (1024*1024) --- 103,108 ---- *************** *** 199,207 **** static const char *show_tcp_keepalives_idle(void); static const char *show_tcp_keepalives_interval(void); static const char *show_tcp_keepalives_count(void); static bool check_maxconnections(int *newval, void **extra, GucSource source); - static void assign_maxconnections(int newval, void *extra); static bool check_autovacuum_max_workers(int *newval, void **extra, GucSource source); - static void assign_autovacuum_max_workers(int newval, void *extra); static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source); static void assign_effective_io_concurrency(int newval, void *extra); static void assign_pgstat_temp_directory(const char *newval, void *extra); --- 188,194 ---- *************** *** 1615,1621 **** static struct config_int ConfigureNamesInt[] = }, &MaxConnections, 100, 1, MAX_BACKENDS, ! check_maxconnections, assign_maxconnections, NULL }, { --- 1602,1608 ---- }, &MaxConnections, 100, 1, MAX_BACKENDS, ! check_maxconnections, NULL, NULL }, { *************** *** 2290,2296 **** static struct config_int ConfigureNamesInt[] = }, &autovacuum_max_workers, 3, 1, MAX_BACKENDS, ! check_autovacuum_max_workers, assign_autovacuum_max_workers, NULL }, { --- 2277,2283 ---- }, &autovacuum_max_workers, 3, 1, MAX_BACKENDS, ! check_autovacuum_max_workers, NULL, NULL }, { *************** *** 8636,8648 **** check_maxconnections(int *newval, void **extra, GucSource source) return true; } - static void - assign_maxconnections(int newval, void *extra) - { - MaxBackends = newval + autovacuum_max_workers + 1 + - GetNumShmemAttachedBgworkers(); - } - static bool check_autovacuum_max_workers(int *newval, void **extra, GucSource source) { --- 8623,8628 ---- *************** *** 8652,8663 **** check_autovacuum_max_workers(int *newval, void **extra, GucSource source) return true; } - static void - assign_autovacuum_max_workers(int newval, void *extra) - { - MaxBackends = MaxConnections + newval + 1 + GetNumShmemAttachedBgworkers(); - } - static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source) { --- 8632,8637 ---- *** a/src/include/postmaster/postmaster.h --- b/src/include/postmaster/postmaster.h *************** *** 61,64 **** extern Size ShmemBackendArraySize(void); --- 61,73 ---- extern void ShmemBackendArrayAllocation(void); #endif + /* + * Note: MAX_BACKENDS is limited to 2^23-1 because inval.c stores the + * backend ID as a 3-byte signed integer. Even if that limitation were + * removed, we still could not exceed INT_MAX/4 because some places compute + * 4*MaxBackends without any overflow check. This is rechecked in the relevant + * GUC check hooks and in RegisterBackgroundWorker(). + */ + #define MAX_BACKENDS 0x7fffff + #endif /* _POSTMASTER_H */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers