From fe75c1e12da061c0ed6f533061aae01d8a58a1e5 Mon Sep 17 00:00:00 2001 From: Valentine Krasnobaeva Date: Tue, 9 Jul 2024 14:03:01 +0200 Subject: [PATCH] MEDIUM: startup: remove MODE_MWORKER_WAIT MODE_MWORKER_WAIT becames redundant with MODE_MWORKER, due to moving master-worker fork in init(). This change allows master no longer perform reexec just after forking in order to free additional memory. As after the fork in the master process we set 'master' variable, we can replace now MODE_MWORKER_WAIT in some 'if' statements by simple check of this 'master' variable. Let's also continue to get rid of HAPROXY_MWORKER_WAIT_ONLY environment variable, as it's no longer needed as well. In cfg_program_postparser(), which is used to check if cmdline is defined to launch a program, we completely remove the check of mode for now, because the master process does not parse the configuration for the moment. 'program' section parsing will be reintroduced in master later in the next commits. --- include/haproxy/global-t.h | 2 +- src/errors.c | 3 +- src/haproxy.c | 71 +++++++++++++++++--------------------- src/http_client.c | 4 +-- src/mworker-prog.c | 6 ---- src/resolvers.c | 2 +- 6 files changed, 37 insertions(+), 51 deletions(-) diff --git a/include/haproxy/global-t.h b/include/haproxy/global-t.h index 8ded609c57..600c9ab212 100644 --- a/include/haproxy/global-t.h +++ b/include/haproxy/global-t.h @@ -35,7 +35,7 @@ #define MODE_STARTING 0x20 #define MODE_FOREGROUND 0x40 #define MODE_MWORKER 0x80 /* Master Worker */ -#define MODE_MWORKER_WAIT 0x100 /* Master Worker wait mode */ +/* (1<<8) unused */ #define MODE_ZERO_WARNING 0x200 /* warnings cause a failure */ #define MODE_DIAG 0x400 /* extra warnings */ #define MODE_CHECK_CONDITION 0x800 /* -cc mode */ diff --git a/src/errors.c b/src/errors.c index 197a0cdefd..f63031f24a 100644 --- a/src/errors.c +++ b/src/errors.c @@ -129,8 +129,7 @@ void startup_logs_init_shm() /* during startup, or just after a reload. * Note: the WAIT_ONLY env variable must be * check in case of an early call */ - if (!(global.mode & MODE_MWORKER_WAIT) && - getenv("HAPROXY_MWORKER_WAIT_ONLY") == NULL) { + if (!(global.mode & MODE_MWORKER)) { if (fd != -1) close(fd); diff --git a/src/haproxy.c b/src/haproxy.c index 7463626413..0115808007 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -776,35 +776,33 @@ static void mworker_reexec(int hardreload) /* insert the new options just after argv[0] in case we have a -- */ - if (getenv("HAPROXY_MWORKER_WAIT_ONLY") == NULL) { - /* add -sf * to argv */ - if (mworker_child_nb() > 0) { - struct mworker_proc *child; + /* add -sf * to argv */ + if (mworker_child_nb() > 0) { + struct mworker_proc *child; - if (hardreload) - next_argv[next_argc++] = "-st"; - else - next_argv[next_argc++] = "-sf"; + if (hardreload) + next_argv[next_argc++] = "-st"; + else + next_argv[next_argc++] = "-sf"; - list_for_each_entry(child, &proc_list, list) { - if (!(child->options & PROC_O_LEAVING) && (child->options & PROC_O_TYPE_WORKER)) - current_child = child; - - if (!(child->options & (PROC_O_TYPE_WORKER|PROC_O_TYPE_PROG)) || child->pid <= -1) - continue; - if ((next_argv[next_argc++] = memprintf(&msg, "%d", child->pid)) == NULL) - goto alloc_error; - msg = NULL; - } - } - if (!x_off && current_child) { - /* add the -x option with the socketpair of the current worker */ - next_argv[next_argc++] = "-x"; - if ((next_argv[next_argc++] = memprintf(&msg, "sockpair@%d", current_child->ipc_fd[0])) == NULL) + list_for_each_entry(child, &proc_list, list) { + if (!(child->options & PROC_O_LEAVING) && (child->options & PROC_O_TYPE_WORKER)) + current_child = child; + + if (!(child->options & (PROC_O_TYPE_WORKER|PROC_O_TYPE_PROG)) || child->pid <= -1) + continue; + if ((next_argv[next_argc++] = memprintf(&msg, "%d", child->pid)) == NULL) goto alloc_error; msg = NULL; } } + if (!x_off && current_child) { + /* add the -x option with the socketpair of the current worker */ + next_argv[next_argc++] = "-x"; + if ((next_argv[next_argc++] = memprintf(&msg, "sockpair@%d", current_child->ipc_fd[0])) == NULL) + goto alloc_error; + msg = NULL; + } if (x_off) { /* if the cmdline contained a -x /dev/null, continue to use it */ @@ -2028,12 +2026,6 @@ static void init(int argc, char **argv) | MODE_DIAG | MODE_CHECK_CONDITION | MODE_DUMP_LIBS | MODE_DUMP_KWD | MODE_DUMP_CFG | MODE_DUMP_NB_L)); - if (getenv("HAPROXY_MWORKER_WAIT_ONLY")) { - unsetenv("HAPROXY_MWORKER_WAIT_ONLY"); - global.mode |= MODE_MWORKER_WAIT; - global.mode &= ~MODE_MWORKER; - } - /* Do check_condition, if we started with -cc, and exit. */ if (global.mode & MODE_CHECK_CONDITION) do_check_condition(progname); @@ -2045,7 +2037,6 @@ static void init(int argc, char **argv) usermsgs_clr("config"); - /* in wait mode, we don't try to read the configuration files */ if (!(global.mode & MODE_MWORKER)) { ret = read_cfg(progname); /* free memory to store config file content */ @@ -2120,7 +2111,7 @@ static void init(int argc, char **argv) /* open log & pid files before the chroot */ if ((global.mode & MODE_DAEMON || global.mode & MODE_MWORKER) && - !(global.mode & MODE_MWORKER_WAIT) && global.pidfile != NULL) { + global.pidfile != NULL) { unlink(global.pidfile); pidfd = open(global.pidfile, O_CREAT | O_WRONLY | O_TRUNC, 0644); if (pidfd < 0) { @@ -2174,7 +2165,6 @@ static void init(int argc, char **argv) break; default: /* in parent */ - global.mode |= MODE_MWORKER_WAIT; master = 1; atexit_flag = 1; atexit(exit_on_failure); @@ -3405,7 +3395,7 @@ int main(int argc, char **argv) * is started and run under the same non-root user, this allows * binding to privileged ports. */ - if (!(global.mode & MODE_MWORKER)) + if (!master) prepare_caps_from_permitted_set(geteuid(), global.uid, argv[0]); #endif @@ -3414,7 +3404,7 @@ int main(int argc, char **argv) * and check mode */ if (old_unixsocket && - !(global.mode & (MODE_MWORKER_WAIT|MODE_CHECK|MODE_CHECK_CONDITION))) { + !(global.mode & (MODE_MWORKER|MODE_CHECK|MODE_CHECK_CONDITION))) { if (strcmp("/dev/null", old_unixsocket) != 0) { if (sock_get_old_sockets(old_unixsocket) != 0) { ha_alert("Failed to get the sockets from the old process!\n"); @@ -3464,7 +3454,7 @@ int main(int argc, char **argv) exit(1); } - if (!(global.mode & MODE_MWORKER_WAIT) && listeners == 0) { + if (!master && listeners == 0) { ha_alert("[%s.main()] No enabled listener found (check for 'bind' directives) ! Exiting.\n", argv[0]); /* Note: we don't have to send anything to the old pids because we * never stopped them. */ @@ -3496,11 +3486,14 @@ int main(int argc, char **argv) } } - if (nb_oldpids && !(global.mode & MODE_MWORKER_WAIT)) + if (nb_oldpids && !master) nb_oldpids = tell_old_pids(oldpids_sig); - /* send a SIGTERM to workers who have a too high reloads number */ - if ((global.mode & MODE_MWORKER) && !(global.mode & MODE_MWORKER_WAIT)) + /* Send a SIGTERM to workers who have a too high reloads number. + * master=1 means that fork() was done before. So, at this stage we already + * have at least one new worker with reloads=0, which is bound to sockets. + */ + if (master) mworker_kill_max_reloads(SIGTERM); /* Note that any error at this stage will be fatal because we will not @@ -3546,7 +3539,7 @@ int main(int argc, char **argv) /* We won't ever use this anymore */ ha_free(&global.pidfile); - if (global.mode & MODE_MWORKER_WAIT) { + if (master) { struct mworker_proc *child, *it; if ((!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)) && diff --git a/src/http_client.c b/src/http_client.c index 5db0e175c8..ff2ab4e85f 100644 --- a/src/http_client.c +++ b/src/http_client.c @@ -1202,7 +1202,7 @@ struct proxy *httpclient_create_proxy(const char *id) struct server *srv_ssl = NULL; #endif - if (global.mode & MODE_MWORKER_WAIT) + if (global.mode & MODE_MWORKER) return ERR_NONE; px = alloc_new_proxy(id, PR_CAP_LISTEN|PR_CAP_INT|PR_CAP_HTTPCLIENT, &errmsg); @@ -1359,7 +1359,7 @@ static int httpclient_postcheck_proxy(struct proxy *curproxy) struct server *srv_ssl = NULL; #endif - if (global.mode & MODE_MWORKER_WAIT) + if (global.mode & MODE_MWORKER) return ERR_NONE; if (!(curproxy->cap & PR_CAP_HTTPCLIENT)) diff --git a/src/mworker-prog.c b/src/mworker-prog.c index 2734d950be..e15dc16e04 100644 --- a/src/mworker-prog.c +++ b/src/mworker-prog.c @@ -115,7 +115,6 @@ int mworker_ext_launch_all() /* This one must not be exported, it's internal! */ unsetenv("HAPROXY_MWORKER_REEXEC"); unsetenv("HAPROXY_STARTUPLOGS_FD"); - unsetenv("HAPROXY_MWORKER_WAIT_ONLY"); unsetenv("HAPROXY_PROCESSES"); execvp(child->command[0], child->command); @@ -332,11 +331,6 @@ int cfg_program_postparser() int err_code = 0; struct mworker_proc *child; - /* we only need to check this during configuration parsing, - * wait mode doesn't have the complete description of a program */ - if (global.mode & MODE_MWORKER_WAIT) - return err_code; - list_for_each_entry(child, &proc_list, list) { if (child->reloads == 0 && (child->options & PROC_O_TYPE_PROG)) { if (child->command == NULL) { diff --git a/src/resolvers.c b/src/resolvers.c index 9e28bf6867..63d3791692 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -3841,7 +3841,7 @@ int resolvers_create_default() { int err_code = ERR_NONE; - if (global.mode & MODE_MWORKER_WAIT) /* does not create the section if in wait mode */ + if (global.mode & MODE_MWORKER) /* does not create the section if in MODE_MWORKER */ return ERR_NONE; /* if the section already exists, do nothing */ -- 2.39.5