145 lines
5.0 KiB
Diff
145 lines
5.0 KiB
Diff
From ff81ac47267c4e0227d1e3fbc5b1dedfd81a2a2f Mon Sep 17 00:00:00 2001
|
|
From: Willy Tarreau <w@1wt.eu>
|
|
Date: Tue, 25 Oct 2016 17:20:24 +0200
|
|
Subject: [PATCH 16/26] BUG/MEDIUM: systemd: let the wrapper know that haproxy
|
|
has completed or failed
|
|
|
|
Pierre Cheynier found that there's a persistent issue with the systemd
|
|
wrapper. Too fast reloads can lead to certain old processes not being
|
|
signaled at all and continuing to run. The problem was tracked down as
|
|
a race between the startup and the signal processing : nothing prevents
|
|
the wrapper from starting new processes while others are still starting,
|
|
and the resulting pid file will only contain the latest pids in this
|
|
case. This can happen with large configs and/or when a lot of SSL
|
|
certificates are involved.
|
|
|
|
In order to solve this we want the wrapper to wait for the new processes
|
|
to complete their startup. But we also want to ensure it doesn't wait for
|
|
nothing in case of error.
|
|
|
|
The solution found here is to create a pipe between the wrapper and the
|
|
sub-processes. The wrapper waits on the pipe and the sub-processes are
|
|
expected to close this pipe once they completed their startup. That way
|
|
we don't queue up new processes until the previous ones have registered
|
|
their pids to the pid file. And if anything goes wrong, the wrapper is
|
|
immediately released. The only thing is that we need the sub-processes
|
|
to know the pipe's file descriptor. We pass it in an environment variable
|
|
called HAPROXY_WRAPPER_FD.
|
|
|
|
It was confirmed both by Pierre and myself that this completely solves
|
|
the "zombie" process issue so that only the new processes continue to
|
|
listen on the sockets.
|
|
|
|
It seems that in the future this stuff could be moved to the haproxy
|
|
master process, also getting rid of an environment variable.
|
|
|
|
This fix needs to be backported to 1.6 and 1.5.
|
|
(cherry picked from commit b957109727f7fed556c049d40bacf45f0df211db)
|
|
---
|
|
src/haproxy-systemd-wrapper.c | 36 ++++++++++++++++++++++++++++++++++++
|
|
src/haproxy.c | 10 ++++++++++
|
|
2 files changed, 46 insertions(+)
|
|
|
|
diff --git a/src/haproxy-systemd-wrapper.c b/src/haproxy-systemd-wrapper.c
|
|
index f4fcab1..b426f96 100644
|
|
--- a/src/haproxy-systemd-wrapper.c
|
|
+++ b/src/haproxy-systemd-wrapper.c
|
|
@@ -65,16 +65,30 @@ static void locate_haproxy(char *buffer, size_t buffer_size)
|
|
return;
|
|
}
|
|
|
|
+/* Note: this function must not exit in case of error (except in the child), as
|
|
+ * it is only dedicated the starting a new haproxy process. By keeping the
|
|
+ * process alive it will ensure that future signal delivery may get rid of
|
|
+ * the issue. If the first startup fails, the wrapper will notice it and
|
|
+ * return an error thanks to wait() returning ECHILD.
|
|
+ */
|
|
static void spawn_haproxy(char **pid_strv, int nb_pid)
|
|
{
|
|
char haproxy_bin[512];
|
|
pid_t pid;
|
|
int main_argc;
|
|
char **main_argv;
|
|
+ int pipefd[2];
|
|
+ char fdstr[20];
|
|
+ int ret;
|
|
|
|
main_argc = wrapper_argc - 1;
|
|
main_argv = wrapper_argv + 1;
|
|
|
|
+ if (pipe(pipefd) != 0) {
|
|
+ fprintf(stderr, SD_NOTICE "haproxy-systemd-wrapper: failed to create a pipe, please try again later.\n");
|
|
+ return;
|
|
+ }
|
|
+
|
|
pid = fork();
|
|
if (!pid) {
|
|
char **argv;
|
|
@@ -89,6 +103,15 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
|
|
}
|
|
|
|
reset_signal_handler();
|
|
+
|
|
+ close(pipefd[0]); /* close the read side */
|
|
+
|
|
+ snprintf(fdstr, sizeof(fdstr), "%d", pipefd[1]);
|
|
+ if (setenv("HAPROXY_WRAPPER_FD", fdstr, 1) != 0) {
|
|
+ fprintf(stderr, SD_NOTICE "haproxy-systemd-wrapper: failed to setenv(), please try again later.\n");
|
|
+ exit(1);
|
|
+ }
|
|
+
|
|
locate_haproxy(haproxy_bin, 512);
|
|
argv[argno++] = haproxy_bin;
|
|
for (i = 0; i < main_argc; ++i)
|
|
@@ -113,6 +136,19 @@ static void spawn_haproxy(char **pid_strv, int nb_pid)
|
|
else if (pid == -1) {
|
|
fprintf(stderr, SD_NOTICE "haproxy-systemd-wrapper: failed to fork(), please try again later.\n");
|
|
}
|
|
+
|
|
+ /* The parent closes the write side and waits for the child to close it
|
|
+ * as well. Also deal the case where the fd would unexpectedly be 1 or 2
|
|
+ * by silently draining all data.
|
|
+ */
|
|
+ close(pipefd[1]);
|
|
+
|
|
+ do {
|
|
+ char c;
|
|
+ ret = read(pipefd[0], &c, sizeof(c));
|
|
+ } while ((ret > 0) || (ret == -1 && errno == EINTR));
|
|
+ /* the child has finished starting up */
|
|
+ close(pipefd[0]);
|
|
}
|
|
|
|
static int read_pids(char ***pid_strv)
|
|
diff --git a/src/haproxy.c b/src/haproxy.c
|
|
index a657dc4..2d476f2 100644
|
|
--- a/src/haproxy.c
|
|
+++ b/src/haproxy.c
|
|
@@ -1843,6 +1843,7 @@ int main(int argc, char **argv)
|
|
int ret = 0;
|
|
int *children = calloc(global.nbproc, sizeof(int));
|
|
int proc;
|
|
+ char *wrapper_fd;
|
|
|
|
/* the father launches the required number of processes */
|
|
for (proc = 0; proc < global.nbproc; proc++) {
|
|
@@ -1879,6 +1880,15 @@ int main(int argc, char **argv)
|
|
close(pidfd);
|
|
}
|
|
|
|
+ /* each child must notify the wrapper that it's ready by closing the requested fd */
|
|
+ wrapper_fd = getenv("HAPROXY_WRAPPER_FD");
|
|
+ if (wrapper_fd) {
|
|
+ int pipe_fd = atoi(wrapper_fd);
|
|
+
|
|
+ if (pipe_fd >= 0)
|
|
+ close(pipe_fd);
|
|
+ }
|
|
+
|
|
/* We won't ever use this anymore */
|
|
free(oldpids); oldpids = NULL;
|
|
free(global.chroot); global.chroot = NULL;
|
|
--
|
|
2.7.3
|
|
|