From 5ee266b745e539e5c21bc093d1dd8353881f22ce Mon Sep 17 00:00:00 2001 From: Valentine Krasnobaeva Date: Mon, 21 Oct 2024 17:10:18 +0200 Subject: [PATCH] MINOR: error: simplify startup_logs_init_shm This patch simplifies the code of startup_logs_init_shm(). We no longer re-exec master process twice after each reload to free its unused memory, which it had to allocate, because it has parsed all configuration sections. So, there is no longer need to keep SHM fd opened between the first and the next reloads. We can completely remove HAPROXY_STARTUPLOGS_FD. In step_init_1() we continue to call startup_logs_init_shm() to open SHM and to allocate startup logs ring area within it. In master-worker mode, worker duplicates initial startup logs ring after sending its READY state to master. Sharing the same ring between two processes until the worker finishes its initialization allows to show at master CLI output worker's startup logs. During the next reload master process should free the memory allocated for the ring structure. Then after the execvp() it will reopen and map SHM area again and it will reallocate again the ring structure. --- src/errors.c | 80 ++++++++++------------------------------------ src/haproxy.c | 3 ++ src/mworker-prog.c | 1 - 3 files changed, 19 insertions(+), 65 deletions(-) diff --git a/src/errors.c b/src/errors.c index f63031f24a..26e5fa0ab6 100644 --- a/src/errors.c +++ b/src/errors.c @@ -102,77 +102,29 @@ error: } /* - * Use a shm across reexec of the master. - * - * During the startup of the master, a shm_open must be done and the FD saved - * into the HAPROXY_STARTUPLOGS_FD environment variable. - * - * When forking workers, the child must use a copy of the shm, not the shm itself. - * - * Once in wait mode, the shm must be copied and closed. - * + * At process start (step_init_1) opens shm and allocates the ring area for the + * startup logs into it. In master-worker mode master and worker share the same + * ring until the moment, when worker sends READY state to master. This is done + * in order to show worker's init logs in master CLI as the output of the + * 'reload' command. After sending its READY status to master, worker must use + * its copy of the shm, not the shm itself. */ -void startup_logs_init_shm() +static struct ring *startup_logs_init_shm() { struct ring *r = NULL; - char *str_fd, *endptr; int fd = -1; - str_fd = getenv("HAPROXY_STARTUPLOGS_FD"); - if (str_fd) { - fd = strtol(str_fd, &endptr, 10); - if (*endptr != '\0') - goto error; - unsetenv("HAPROXY_STARTUPLOGS_FD"); - } - - /* 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)) { - if (fd != -1) - close(fd); - - fd = startup_logs_new_shm(); - if (fd == -1) - goto error; - - r = startup_logs_from_fd(fd, 1); - if (!r) - goto error; - - str_fd = NULL; - memprintf(&str_fd, "%d", fd); - setenv("HAPROXY_STARTUPLOGS_FD", str_fd, 1); - ha_free(&str_fd); - - } else { - /* in wait mode, copy the shm to an allocated buffer */ - struct ring *prev = NULL; - - if (fd == -1) - goto error; - - prev = startup_logs_from_fd(fd, 0); - if (!prev) - goto error; - - r = startup_logs_dup(prev); - if (!r) - goto error; - startup_logs_free(prev); - close(fd); - } + fd = startup_logs_new_shm(); + if (fd == -1) + return NULL; - startup_logs = r; + r = startup_logs_from_fd(fd, 1); + close(fd); - return; -error: - if (fd != -1) - close(fd); - /* couldn't get a mmap to work */ - startup_logs = ring_new(STARTUP_LOG_SIZE); + if (!r) + return NULL; + return r; } #endif /* ! USE_SHM_OPEN */ @@ -180,7 +132,7 @@ error: void startup_logs_init() { #ifdef USE_SHM_OPEN - startup_logs_init_shm(); + startup_logs = startup_logs_init_shm(); #else /* ! USE_SHM_OPEN */ startup_logs = ring_new(STARTUP_LOG_SIZE); #endif diff --git a/src/haproxy.c b/src/haproxy.c index e8b688f1ee..8aa93c601f 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -753,6 +753,8 @@ static void mworker_reexec(int hardreload) /* ensure that we close correctly every listeners before reexecuting */ mworker_cleanlisteners(); + startup_logs_free(startup_logs); + /* during the reload we must ensure that every FDs that can't be * reuse (ie those that are not referenced in the proc_list) * are closed or they will leak. */ @@ -2308,6 +2310,7 @@ static void step_init_1() #endif #endif /* USE_OPENSSL */ + /* saves ptr to ring in startup_logs var */ startup_logs_init(); if (init_acl() != 0) diff --git a/src/mworker-prog.c b/src/mworker-prog.c index 8f745c7c1e..4cf0da3cc2 100644 --- a/src/mworker-prog.c +++ b/src/mworker-prog.c @@ -118,7 +118,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_PROCESSES"); execvp(child->command[0], child->command); -- 2.39.5