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).

Author: m6w6

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();
+	}
 }
 /* }}} */
 

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);

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();

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();

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();

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();

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();