diff --git a/src/Core/Retry/CommandRetry.php b/src/Core/Retry/CommandRetry.php new file mode 100644 index 00000000000..cbbdf828ec1 --- /dev/null +++ b/src/Core/Retry/CommandRetry.php @@ -0,0 +1,80 @@ +strategy = $strategy; + $this->retries = $retries; + } + + /** + * The number of retries to perform in case of failure + * + * @param callable $action The callable action to execute with a retry strategy + * @return mixed The return value of the passed action callable + */ + public function run(callable $action) + { + $retryCount = 0; + $lastException = null; + while ($this->retries > $retryCount) { + $retryCount++; + try { + return $action(); + } catch (Exception $e) { + $lastException = $e; + if (!$this->strategy->shouldRetry($e, $retryCount)) { + throw $e; + } + } + } + + if ($lastException !== null) { + throw $lastException; + } + } +} diff --git a/src/Core/Retry/RetryStrategyInterface.php b/src/Core/Retry/RetryStrategyInterface.php new file mode 100644 index 00000000000..ae3ce198dd1 --- /dev/null +++ b/src/Core/Retry/RetryStrategyInterface.php @@ -0,0 +1,33 @@ +_driver->prepare($sql); + return $this->getDisconnectRetry()->run(function () use ($sql) { + $statement = $this->_driver->prepare($sql); - if ($this->_logQueries) { - $statement = $this->_newLogger($statement); - } + if ($this->_logQueries) { + $statement = $this->_newLogger($statement); + } - return $statement; + return $statement; + }); } /** @@ -278,15 +293,17 @@ public function prepare($sql) */ public function execute($query, array $params = [], array $types = []) { - if (!empty($params)) { - $statement = $this->prepare($query); - $statement->bind($params, $types); - $statement->execute(); - } else { - $statement = $this->query($query); - } + return $this->getDisconnectRetry()->run(function () use ($query, $params, $types) { + if (!empty($params)) { + $statement = $this->prepare($query); + $statement->bind($params, $types); + $statement->execute(); + } else { + $statement = $this->query($query); + } - return $statement; + return $statement; + }); } /** @@ -311,11 +328,13 @@ public function compileQuery(Query $query, ValueBinder $generator) */ public function run(Query $query) { - $statement = $this->prepare($query); - $query->getValueBinder()->attachTo($statement); - $statement->execute(); + return $this->getDisconnectRetry()->run(function () use ($query) { + $statement = $this->prepare($query); + $query->getValueBinder()->attachTo($statement); + $statement->execute(); - return $statement; + return $statement; + }); } /** @@ -326,10 +345,12 @@ public function run(Query $query) */ public function query($sql) { - $statement = $this->prepare($sql); - $statement->execute(); + return $this->getDisconnectRetry()->run(function () use ($sql) { + $statement = $this->prepare($sql); + $statement->execute(); - return $statement; + return $statement; + }); } /** @@ -403,12 +424,14 @@ public function schemaCollection(SchemaCollection $collection = null) */ public function insert($table, array $data, array $types = []) { - $columns = array_keys($data); + return $this->getDisconnectRetry()->run(function () use ($table, $data, $types) { + $columns = array_keys($data); - return $this->newQuery()->insert($columns, $types) - ->into($table) - ->values($data) - ->execute(); + return $this->newQuery()->insert($columns, $types) + ->into($table) + ->values($data) + ->execute(); + }); } /** @@ -422,10 +445,12 @@ public function insert($table, array $data, array $types = []) */ public function update($table, array $data, array $conditions = [], $types = []) { - return $this->newQuery()->update($table) - ->set($data, $types) - ->where($conditions, $types) - ->execute(); + return $this->getDisconnectRetry()->run(function () use ($table, $data, $conditions, $types) { + return $this->newQuery()->update($table) + ->set($data, $types) + ->where($conditions, $types) + ->execute(); + }); } /** @@ -438,9 +463,11 @@ public function update($table, array $data, array $conditions = [], $types = []) */ public function delete($table, $conditions = [], $types = []) { - return $this->newQuery()->delete($table) - ->where($conditions, $types) - ->execute(); + return $this->getDisconnectRetry()->run(function () use ($table, $conditions, $types) { + return $this->newQuery()->delete($table) + ->where($conditions, $types) + ->execute(); + }); } /** @@ -454,7 +481,11 @@ public function begin() if ($this->_logQueries) { $this->log('BEGIN'); } - $this->_driver->beginTransaction(); + + $this->getDisconnectRetry()->run(function () { + $this->_driver->beginTransaction(); + }); + $this->_transactionLevel = 0; $this->_transactionStarted = true; $this->nestedTransactionRollbackException = null; @@ -648,7 +679,9 @@ public function rollbackSavepoint($name) */ public function disableForeignKeys() { - $this->execute($this->_driver->disableForeignKeySQL())->closeCursor(); + $this->getDisconnectRetry()->run(function () { + $this->execute($this->_driver->disableForeignKeySQL())->closeCursor(); + }); } /** @@ -658,7 +691,9 @@ public function disableForeignKeys() */ public function enableForeignKeys() { - $this->execute($this->_driver->enableForeignKeySQL())->closeCursor(); + $this->getDisconnectRetry()->run(function () { + $this->execute($this->_driver->enableForeignKeySQL())->closeCursor(); + }); } /** @@ -733,18 +768,20 @@ protected function wasNestedTransactionRolledback() */ public function disableConstraints(callable $callback) { - $this->disableForeignKeys(); + return $this->getDisconnectRetry()->run(function () use ($callback) { + $this->disableForeignKeys(); - try { - $result = $callback($this); - } catch (Exception $e) { - $this->enableForeignKeys(); - throw $e; - } + try { + $result = $callback($this); + } catch (Exception $e) { + $this->enableForeignKeys(); + throw $e; + } - $this->enableForeignKeys(); + $this->enableForeignKeys(); - return $result; + return $result; + }); } /** diff --git a/src/Database/Retry/ReconnectStrategy.php b/src/Database/Retry/ReconnectStrategy.php new file mode 100644 index 00000000000..aa48710f61d --- /dev/null +++ b/src/Database/Retry/ReconnectStrategy.php @@ -0,0 +1,120 @@ +connection = $connection; + } + + /** + * Checks whether or not the exception was caused by a lost connection, + * and returns true if it was able to successfully reconnect. + * + * @param Exception $exception The exception to check for its message + * @param int $retryCount The number of times the action has been already called + * @return bool Whether or not it is OK to retry the action + */ + public function shouldRetry(Exception $exception, $retryCount) + { + $message = $exception->getMessage(); + + foreach (static::$causes as $cause) { + if (strstr($message, $cause) !== false) { + return $this->reconnect(); + } + } + + return false; + } + + /** + * Tries to re-establish the connection to the server, if it is safe to do so + * + * @return bool Whether or not the connection was re-established + */ + protected function reconnect() + { + if ($this->connection->inTransaction()) { + // It is not safe to blindly reconnect in the middle of a transaction + return false; + } + + try { + // Make sure we free any resources associated with the old connection + $this->connection->disconnect(); + } catch (Exception $e) { + } + + try { + $this->connection->connect(); + $this->connection->log('[RECONNECT]'); + + return true; + } catch (Exception $e) { + // If there was an error connecting again, don't report it back, + // let the retry handler do it. + return false; + } + } +} diff --git a/tests/TestCase/Core/Retry/CommandRetryTest.php b/tests/TestCase/Core/Retry/CommandRetryTest.php new file mode 100644 index 00000000000..43d50393f65 --- /dev/null +++ b/tests/TestCase/Core/Retry/CommandRetryTest.php @@ -0,0 +1,110 @@ +getMockBuilder(RetryStrategyInterface::class)->getMock(); + $strategy + ->expects($this->exactly(3)) + ->method('shouldRetry') + ->will($this->returnCallback(function ($e, $c) use ($exception, &$count) { + $this->assertSame($e, $exception); + $this->assertEquals($c, $count); + + return true; + })); + + $retry = new CommandRetry($strategy, 5); + $retry->run($action); + } + + /** + * Test attempts exceeded + * + * @return void + */ + public function testExceedAttempts() + { + $exception = new Exception('this is failing'); + $action = function () use ($exception) { + throw $exception; + }; + + $strategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); + $strategy + ->expects($this->exactly(3)) + ->method('shouldRetry') + ->will($this->returnCallback(function ($e) use ($exception) { + return true; + })); + + $retry = new CommandRetry($strategy, 3); + $this->expectException(Exception::class); + $this->expectExceptionMessage('this is failing'); + $retry->run($action); + } + /** + * Test that the strategy is respected + * + * @return void + */ + public function testRespectStrategy() + { + $action = function () { + throw new Exception('this is failing'); + }; + + $strategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); + $strategy + ->expects($this->once()) + ->method('shouldRetry') + ->will($this->returnCallback(function () { + return false; + })); + + $retry = new CommandRetry($strategy, 3); + $this->expectException(Exception::class); + $this->expectExceptionMessage('this is failing'); + $retry->run($action); + } +} diff --git a/tests/TestCase/Database/ConnectionTest.php b/tests/TestCase/Database/ConnectionTest.php index 00eb1123deb..772ae8f3fa1 100644 --- a/tests/TestCase/Database/ConnectionTest.php +++ b/tests/TestCase/Database/ConnectionTest.php @@ -20,10 +20,14 @@ use Cake\Database\Exception\NestedTransactionRollbackException; use Cake\Database\Log\LoggingStatement; use Cake\Database\Log\QueryLogger; +use Cake\Database\Retry\CommandRetry; +use Cake\Database\Retry\ReconnectStrategy; use Cake\Datasource\ConnectionManager; use Cake\Log\Log; use Cake\TestSuite\TestCase; +use Exception; use ReflectionMethod; +use ReflectionProperty; /** * Tests Connection class @@ -1215,4 +1219,65 @@ public function pushNestedTransactionState() $method->setAccessible(true); $this->nestedTransactionStates[] = $method->invoke($this->connection); } + + /** + * Tests that the connection is restablished whenever it is interrupted + * after having used the connection at least once. + * + * @return void + */ + public function testAutomaticReconnect() + { + $conn = clone $this->connection; + $statement = $conn->query('SELECT 1'); + $statement->execute(); + $statement->closeCursor(); + + $prop = new ReflectionProperty($conn, '_driver'); + $prop->setAccessible(true); + $oldDriver = $prop->getValue($conn); + $newDriver = $this->getMockBuilder('Cake\Database\Driver')->getMock(); + $prop->setValue($conn, $newDriver); + + $newDriver->expects($this->at(0)) + ->method('prepare') + ->will($this->throwException(new Exception('server gone away'))); + + $newDriver->expects($this->at(1))->method('disconnect'); + $newDriver->expects($this->at(2))->method('connect'); + $newDriver->expects($this->at(3)) + ->method('prepare') + ->will($this->returnValue($statement)); + + $this->assertSame($statement, $conn->query('SELECT 1')); + } + + /** + * Tests that the connection is not restablished whenever it is interrupted + * inside a transaction. + * + * @return void + */ + public function testNoAutomaticReconnect() + { + $conn = clone $this->connection; + $statement = $conn->query('SELECT 1'); + $statement->execute(); + $statement->closeCursor(); + + $conn->begin(); + + $prop = new ReflectionProperty($conn, '_driver'); + $prop->setAccessible(true); + $oldDriver = $prop->getValue($conn); + $newDriver = $this->getMockBuilder('Cake\Database\Driver')->getMock(); + $prop->setValue($conn, $newDriver); + + $newDriver->expects($this->once()) + ->method('prepare') + ->will($this->throwException(new Exception('server gone away'))); + + $this->expectException(Exception::class); + $conn->query('SELECT 1'); + } }