From f8a35d758ab4599c8465d76c235ab3ba49a62bbe Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 15 Mar 2019 18:23:04 +0100 Subject: [PATCH 1/2] Fix #265 Recover from Lambda timeouts In #257 I added some code to support and recover from PHP-FPM timeouts. However we still have problems with Lambda timeouts (#265): those timeouts cause AWS Lambda to kill our `bootstrap` process and restart it. I added some code to handle all scenarios I could think of: whether PHP-FPM was still running or not. From what I've seen in my tests Lambda correctly kills the `bootstrap` process but it kills PHP-FPM properly as well. That's good, in those cases Bref will simply restart PHP-FPM and be good to go. I cannot add regression tests because SAM recreates a new instance on every invocation, so it's impossible to keep a broken instance around. I tested directly on AWS Lambda. This change requires a new layer version because `php-fpm.conf` is modified. --- demo/http.php | 19 ++------ runtime/php/layers/fpm/php-fpm.conf | 1 + src/Runtime/PhpFpm.php | 71 +++++++++++++++++++++++++++-- template.yaml | 2 +- tests/Runtime/PhpFpmTest.php | 5 +- 5 files changed, 78 insertions(+), 20 deletions(-) diff --git a/demo/http.php b/demo/http.php index cbb29ff60..5baae8b64 100644 --- a/demo/http.php +++ b/demo/http.php @@ -1,20 +1,9 @@ get('/', function (ServerRequestInterface $request, ResponseInterface $response) { - $response->getBody()->write('Hello world!'); - return $response; -}); -$slim->get('/json', function (ServerRequestInterface $request, ResponseInterface $response) { - $response->getBody()->write(json_encode(['hello' => 'json'])); - $response = $response->withHeader('Content-Type', 'application/json'); - return $response; -}); +if (isset($_GET['sleep'])) { + sleep(10); +} -$slim->run(); +echo 'Hello world!'; diff --git a/runtime/php/layers/fpm/php-fpm.conf b/runtime/php/layers/fpm/php-fpm.conf index fad05e1e2..19dab1f53 100644 --- a/runtime/php/layers/fpm/php-fpm.conf +++ b/runtime/php/layers/fpm/php-fpm.conf @@ -1,5 +1,6 @@ ; Logging anywhere on disk doesn't make sense on lambda since instances are ephemeral error_log = /dev/null +pid = /tmp/.bref/php-fpm.pid ; Log above warning because PHP-FPM logs useless notices ; We must comment this flag else uncaught exceptions/fatal errors are not reported in the logs! ; TODO: report that to the PHP bug tracker diff --git a/src/Runtime/PhpFpm.php b/src/Runtime/PhpFpm.php index 057556d0f..9b0669755 100644 --- a/src/Runtime/PhpFpm.php +++ b/src/Runtime/PhpFpm.php @@ -24,6 +24,7 @@ class PhpFpm { private const SOCKET = '/tmp/.bref/php-fpm.sock'; + private const PID_FILE = '/tmp/.bref/php-fpm.pid'; private const CONFIG = '/opt/bref/etc/php-fpm.conf'; /** @var Client|null */ @@ -46,8 +47,10 @@ public function __construct(string $handler, string $configFile = self::CONFIG) */ public function start(): void { + // In case Lambda stopped our process (e.g. because of a timeout) we need to make sure PHP-FPM has stopped + // as well and restart it if ($this->isReady()) { - throw new \Exception('PHP-FPM has already been started, aborting'); + $this->killExistingFpm(); } if (! is_dir(dirname(self::SOCKET))) { @@ -67,7 +70,7 @@ public function start(): void $this->reconnect(); - $this->waitForServerReady(); + $this->waitUntilReady(); } public function stop(): void @@ -140,7 +143,7 @@ public function proxy($event): LambdaResponse return new LambdaResponse((int) $status, $responseHeaders, $responseBody); } - private function waitForServerReady(): void + private function waitUntilReady(): void { $wait = 5000; // 5ms $timeout = 5000000; // 5 secs @@ -252,4 +255,66 @@ private function reconnect(): void $this->client = new Client('unix://' . self::SOCKET, 30000); } + + /** + * This methods makes sure to kill any existing PHP-FPM process. + */ + private function killExistingFpm(): void + { + // Never seen this happen but just in case + if (! file_exists(self::PID_FILE)) { + unlink(self::SOCKET); + return; + } + + $pid = (int) file_get_contents(self::PID_FILE); + + // Never seen this happen but just in case + if ($pid <= 0) { + echo "PHP-FPM's PID file contained an invalid PID, assuming PHP-FPM isn't running.\n"; + unlink(self::SOCKET); + unlink(self::PID_FILE); + return; + } + + // Check if the process is running + if (posix_getpgid($pid) === false) { + // PHP-FPM is not running anymore, we can cleanup + unlink(self::SOCKET); + unlink(self::PID_FILE); + return; + } + + echo "PHP-FPM seems to be running already, this might be because Lambda stopped the bootstrap process but didn't leave us an opportunity to stop PHP-FPM. Stopping PHP-FPM now to restart from a blank slate.\n"; + + // PHP-FPM is running, let's try to kill it properly + $result = posix_kill($pid, SIGTERM); + if ($result === false) { + echo "PHP-FPM's PID file contained a PID that doesn't exist, assuming PHP-FPM isn't running.\n"; + unlink(self::SOCKET); + unlink(self::PID_FILE); + return; + } + + $this->waitUntilStopped($pid); + unlink(self::SOCKET); + unlink(self::PID_FILE); + } + + /** + * Wait until PHP-FPM has stopped. + */ + private function waitUntilStopped(int $pid): void + { + $wait = 5000; // 5ms + $timeout = 1000000; // 1 sec + $elapsed = 0; + while (posix_getpgid($pid) !== false) { + usleep($wait); + $elapsed += $wait; + if ($elapsed > $timeout) { + throw new \Exception('Timeout while waiting for PHP-FPM to stop'); + } + } + } } diff --git a/template.yaml b/template.yaml index c895bc937..ad9f57c54 100644 --- a/template.yaml +++ b/template.yaml @@ -25,7 +25,7 @@ Resources: Description: 'Bref HTTP demo' CodeUri: . Handler: demo/http.php - Timeout: 30 # in seconds (API Gateway has a timeout of 30 seconds) + Timeout: 5 # in seconds (API Gateway has a timeout of 30 seconds) MemorySize: 1024 # The memory size is related to the pricing and CPU power Runtime: provided Layers: diff --git a/tests/Runtime/PhpFpmTest.php b/tests/Runtime/PhpFpmTest.php index 2d6cfae10..95958ab4b 100644 --- a/tests/Runtime/PhpFpmTest.php +++ b/tests/Runtime/PhpFpmTest.php @@ -701,7 +701,10 @@ public function test response with error_log() ], $response['headers']); } - public function test timeouts are recovered from() + /** + * Checks that a timeout cause by the PHP-FPM limit (not the Lambda limit) can be recovered from + */ + public function test FPM timeouts are recovered from() { $this->fpm = new PhpFpm(__DIR__ . '/PhpFpm/timeout.php', __DIR__ . '/PhpFpm/php-fpm.conf'); $this->fpm->start(); From fc2c142c3b51d5da72f105ec161bcf8e59e28c06 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 15 Mar 2019 18:41:49 +0100 Subject: [PATCH 2/2] Workaround a phpstan bug Reported in https://github.com/phpstan/phpstan/pull/2002 --- phpstan.neon.dist | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1510e375d..1542553e2 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -12,3 +12,7 @@ parameters: - %rootDir%/../../../tests/Bridge/Symfony/logs/* - %rootDir%/../../../tests/Sam/Php/* - %rootDir%/../../../tests/Sam/PhpFpm/* + + ignoreErrors: + # https://github.com/phpstan/phpstan/pull/2002 + - '#Strict comparison using === between int and false will always evaluate to false#'