From 372703cf608961ed15652ec53779b263825b9acf Mon Sep 17 00:00:00 2001 From: Michael Wallner Date: Tue, 22 Jan 2019 10:44:50 +0100 Subject: [PATCH] php-fpm: graceful restart without blocking/losing requests ## Issue: When php-fpm gets "graceful" reloaded it will stop accepting requests while finishing reqests it already received. If those requests to finish are of a long running kind like big uploads over a slow connection, this will block new requests to be handled for an unacceptable long time. ### Current behavior ``` | v FPM (OLD) running | v $ sudo kill -USR2 $(cat /run/php-fpm/php-fpm.pid) | v OLD sends SIGQUIT to children v OLD stops handling requests | v OLD keeps running until all children complete or time out | v OLD exec()s v FPM (NEW) starts & re-uses listening socket | v NEW handles incoming requests from now on ``` ## Proposed Solution: In the case of a reloading request, we immediately fork and exec a new fpm instance to seamlessly handle new requests, while the old instance keeps finishing those requests already accepted. ### New behavior ``` | v FPM (OLD) running | v $ sudo kill -USR2 $(cat /run/php-fpm/php-fpm.pid) | v OLD sends SIGQUIT to children v OLD fork()s & exec()s |\ | \ | v FPM (NEW) starts & re-uses listening socket | v NEW handles incoming requests from now on | | v | OLD keeps running until all children complete or time out v | OLD gone - | / / v NEW still going | ``` We've been running this patch since PHP 5.6 (early 2017). --- sapi/fpm/fpm/fpm_process_ctl.c | 27 ++++++++++++++----- sapi/fpm/fpm/fpm_sockets.c | 12 ++++++++- sapi/fpm/tests/bug68442-signal-reload.phpt | 2 ++ .../fpm/tests/bug74083-concurrent-reload.phpt | 2 ++ .../tests/bug76601-reload-child-signals.phpt | 2 ++ .../bug77934-reload-process-control.phpt | 3 +++ .../reload-uses-sigkill-as-last-measure.phpt | 3 +++ 7 files changed, 43 insertions(+), 8 deletions(-) diff --git a/sapi/fpm/fpm/fpm_process_ctl.c b/sapi/fpm/fpm/fpm_process_ctl.c index 2bc00178ea888..a2ed79915e1c0 100644 --- a/sapi/fpm/fpm/fpm_process_ctl.c +++ b/sapi/fpm/fpm/fpm_process_ctl.c @@ -77,11 +77,16 @@ static void fpm_pctl_exit() /* {{{ */ static void fpm_pctl_exec() /* {{{ */ { - zlog(ZLOG_DEBUG, "Blocking some signals before reexec"); - if (0 > fpm_signals_block()) { - zlog(ZLOG_WARNING, "concurrent reloads may be unstable"); + switch (fork()) { + case 0: + break; + case -1: + zlog(ZLOG_SYSERROR, "failed to reload: fork() failed"); + /* no break */ + default: + fpm_global_config.pid_file = NULL; + return; } - zlog(ZLOG_NOTICE, "reloading: execvp(\"%s\", {\"%s\"" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" "%s%s%s" @@ -99,6 +104,11 @@ static void fpm_pctl_exec() /* {{{ */ optional_arg(10) ); + zlog(ZLOG_DEBUG, "Blocking some signals before reexec"); + if (0 > fpm_signals_block()) { + zlog(ZLOG_WARNING, "concurrent reloads may be unstable"); + } + fpm_cleanups_run(FPM_CLEANUP_PARENT_EXEC); execvp(saved_argv[0], saved_argv); zlog(ZLOG_SYSERROR, "failed to reload: execvp() failed"); @@ -110,9 +120,8 @@ static void fpm_pctl_action_last() /* {{{ */ { switch (fpm_state) { case FPM_PCTL_STATE_RELOADING: - fpm_pctl_exec(); - break; - + zlog(ZLOG_NOTICE, "exiting after reload"); + exit(FPM_EXIT_OK); case FPM_PCTL_STATE_FINISHING: case FPM_PCTL_STATE_TERMINATING: fpm_pctl_exit(); @@ -199,6 +208,10 @@ static void fpm_pctl_action_next() /* {{{ */ fpm_pctl_kill_all(sig); fpm_signal_sent = sig; fpm_pctl_timeout_set(timeout); + + if (fpm_signal_sent == SIGQUIT && fpm_state == FPM_PCTL_STATE_RELOADING) { + fpm_pctl_exec(); + } } /* }}} */ diff --git a/sapi/fpm/fpm/fpm_sockets.c b/sapi/fpm/fpm/fpm_sockets.c index aaa16cc89e88f..d12eb130b3937 100644 --- a/sapi/fpm/fpm/fpm_sockets.c +++ b/sapi/fpm/fpm/fpm_sockets.c @@ -74,7 +74,17 @@ static void fpm_sockets_cleanup(int which, void *arg) /* {{{ */ if (which == FPM_CLEANUP_PARENT_EXIT_MAIN) { if (ls->type == FPM_AF_UNIX) { - unlink(ls->key); + struct sockaddr_un sa_un; + + memset(&sa_un, 0, sizeof(sa_un)); + strlcpy(sa_un.sun_path, ls->key, sizeof(sa_un.sun_path)); + sa_un.sun_family = AF_UNIX; + + if (fpm_socket_unix_test_connect(&sa_un, sizeof(sa_un)) == 0) { + zlog(ZLOG_WARNING, "Keeping unix socket, another FPM instance seems to already listen on %s", ls->key); + } else { + unlink(ls->key); + } } } free(ls->key); diff --git a/sapi/fpm/tests/bug68442-signal-reload.phpt b/sapi/fpm/tests/bug68442-signal-reload.phpt index d15c8e14e765c..919e9b1181f5d 100644 --- a/sapi/fpm/tests/bug68442-signal-reload.phpt +++ b/sapi/fpm/tests/bug68442-signal-reload.phpt @@ -29,9 +29,11 @@ $tester->ping('{{ADDR}}'); $tester->signal('USR2'); $tester->expectLogNotice('Reloading in progress ...'); $tester->expectLogNotice('reloading: .*'); +$tester->expectLogNotice('exiting after reload'); $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"'); $tester->expectLogStartNotices(); $tester->ping('{{ADDR}}'); +$tester->signal('TERM'); $tester->terminate(); $tester->expectLogTerminatingNotices(); $tester->close(); diff --git a/sapi/fpm/tests/bug74083-concurrent-reload.phpt b/sapi/fpm/tests/bug74083-concurrent-reload.phpt index 8b4b690e96201..16f85a6a54f22 100644 --- a/sapi/fpm/tests/bug74083-concurrent-reload.phpt +++ b/sapi/fpm/tests/bug74083-concurrent-reload.phpt @@ -57,10 +57,12 @@ $tester->getLogLines(2000); $tester->signal('USR2'); $tester->expectLogNotice('Reloading in progress ...'); $tester->expectLogNotice('reloading: .*'); +$tester->expectLogNotice('exiting after reload'); $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"'); $tester->expectLogStartNotices(); $tester->ping('{{ADDR}}'); +$tester->signal('TERM'); $tester->terminate(); $tester->expectLogTerminatingNotices(); $tester->close(); diff --git a/sapi/fpm/tests/bug76601-reload-child-signals.phpt b/sapi/fpm/tests/bug76601-reload-child-signals.phpt index f095ed667ca83..69c9454d91392 100644 --- a/sapi/fpm/tests/bug76601-reload-child-signals.phpt +++ b/sapi/fpm/tests/bug76601-reload-child-signals.phpt @@ -73,9 +73,11 @@ if (!$tester->expectLogNotice('reloading: .*')) { echo "Skipped messages\n"; echo implode('', $skipped); } +$tester->expectLogNotice('exiting after reload'); $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"'); $tester->expectLogStartNotices(); +$tester->signal('TERM'); $tester->terminate(); $tester->expectLogTerminatingNotices(); $tester->close(); diff --git a/sapi/fpm/tests/bug77934-reload-process-control.phpt b/sapi/fpm/tests/bug77934-reload-process-control.phpt index e7da04421bd9f..cd3c46237ffec 100644 --- a/sapi/fpm/tests/bug77934-reload-process-control.phpt +++ b/sapi/fpm/tests/bug77934-reload-process-control.phpt @@ -30,9 +30,12 @@ $tester->ping('{{ADDR}}'); $tester->signal('USR2'); $tester->expectLogNotice('Reloading in progress ...'); $tester->expectLogNotice('reloading: .*'); +$tester->expectLogNotice('exiting after reload'); $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"'); $tester->expectLogStartNotices(); $tester->ping('{{ADDR}}'); + +$tester->signal('TERM'); $tester->terminate(); $tester->expectLogTerminatingNotices(); $tester->close(); diff --git a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt index 4e2da76d56203..729758f4b034b 100644 --- a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt +++ b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt @@ -45,6 +45,9 @@ sleep(2); $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"'); $tester->expectLogStartNotices(); $tester->ping('{{ADDR}}'); +$tester->expectLogNotice('exiting after reload'); + +$tester->signal('TERM'); $tester->terminate(); $tester->expectLogTerminatingNotices(); $tester->close();