From ea2b62d947b393edc61daa858af17a5d7ec5a4f0 Mon Sep 17 00:00:00 2001 From: Mathieu Rochette Date: Wed, 27 May 2015 17:04:52 +0200 Subject: [PATCH 1/2] add tests for Too many connections --- features/bootstrap/FeatureContext.php | 183 ++++++++++++---------- features/bootstrap/MysqliContext.php | 11 +- features/bootstrap/PdoMysqlContext.php | 17 +- features/retry-master-slave.feature | 6 +- features/retry.feature | 10 ++ src/Driver/Connection/RetryConnection.php | 1 - src/RetryStrategy.php | 1 - src/RetryStrategy/MysqlRetryStrategy.php | 83 +++++----- 8 files changed, 166 insertions(+), 146 deletions(-) diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index 23fb7d9..0b804e6 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -19,6 +19,11 @@ public function clearConnections() 'last-error' => null, ] ]; + $connection = $this->rootConnection(); + $connection->exec("SET GLOBAL MAX_CONNECTIONS = 50"); + $connection->close(); + $connection = null; + gc_collect_cycles(); } /** @@ -26,13 +31,43 @@ public function clearConnections() */ public function clearDatabase() { - $params = $this->masterParams(); - $dbname = $params['dbname']; - unset($params['dbname']); - $connection = DriverManager::getConnection($params); + $dbname = $this->masterParams()['dbname']; + $connection = $this->rootConnection(); $connection->exec("DROP DATABASE IF EXISTS $dbname"); $connection->exec("CREATE DATABASE $dbname"); + $connection->close(); + $connection = null; + gc_collect_cycles(); + } + + /** + * @BeforeScenario + */ + public function assertNoActiveConnection() + { + $n = $this->activeConnectionsCount(); + assert($n === 0, "There is $n active connection(s) on the test server"); + } + + private function activeConnectionsCount() + { + $connection = $this->rootConnection(); + gc_collect_cycles(); + $n = (int)$connection->fetchAll("show status like 'Threads_connected'")[0]['Value']; + $connection->close(); + $connection = null; + gc_collect_cycles(); + return $n - 1; + } + + private function rootConnection() + { + $params = $this->masterParams('root'); + $dbname = $params['dbname']; + unset($params['dbname']); + return DriverManager::getConnection($params); } + /** * @AfterScenario */ @@ -43,6 +78,22 @@ public function closeConnections() $instance->close(); } } + $this->connections = []; + gc_collect_cycles(); + } + + /** + * @Given the server accept :n more connection + * @Given the server accept :n more connections + */ + public function theServerAcceptMoreConnections($n) + { + $n += $this->activeConnectionsCount(); + $connection = $this->rootConnection(); + $connection->exec("SET GLOBAL MAX_CONNECTIONS = $n"); + $connection->close(); + $connection = null; + gc_collect_cycles(); } /** @@ -75,43 +126,12 @@ public function aMasterSlavesConnectionWithSlaves($connectionName, $slaveCount) /** * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retries + * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry with username :username + * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retries with username :username */ - public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetries($connectionName, $slaveCount, $n) - { - $master = $this->masterParams(); - - $slaveCount = (int) $slaveCount; - $slaves = []; - while ($slaveCount--) { - $master['weight'] = 1; - $slaves[] = $master; - } - - $params = [ - 'driverClass' => 'Ez\DbLinker\Driver\MysqlRetryDriver', - 'connectionParams' => [ - 'master' => $master, - 'slaves' => $slaves, - 'driverClass' => 'Ez\DbLinker\Driver\MysqlMasterSlavesDriver', - ], - 'retryStrategy' => new MysqlRetryStrategy($n), - ]; - $this->connections[$connectionName] = [ - 'params' => $params, - 'instance' => null, - 'last-result' => null, - 'last-error' => null, - ]; - } - - /** - * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry with username :userName - * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retries with username :userName - */ - public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetriesWithUsername($connectionName, $slaveCount, $n, $userName) + public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetriesWithusername($connectionName, $slaveCount, $n, $username = null) { - $master = $this->masterParams(); - $master['user'] = $userName; + $master = $this->masterParams($username); $slaveCount = (int) $slaveCount; $slaves = []; @@ -238,34 +258,15 @@ public function aTransactionIsStartedOn($connectionName) /** * @Given a retry connection :connectionName limited to :n retry - * @Given a retry connection :connectionName limited to :n retries - */ - public function aRetryConnection($connectionName, $n) - { - $params = [ - 'driverClass' => 'Ez\DbLinker\Driver\MysqlRetryDriver', - 'connectionParams' => $this->masterParams(), - 'retryStrategy' => new MysqlRetryStrategy($n), - ]; - $this->connections[$connectionName] = [ - 'params' => $params, - 'instance' => null, - 'last-result' => null, - 'last-error' => null, - ]; - } - - /** * @Given a retry connection :connectionName limited to :n retry with username :username */ - public function aRetryConnectionLimitedToRetryWithUsername($connectionName, $n, $username) + public function aRetryConnectionLimitedToRetryWithusername($connectionName, $n, $username = null) { $params = [ 'driverClass' => 'Ez\DbLinker\Driver\MysqlRetryDriver', - 'connectionParams' => $this->masterParams(), + 'connectionParams' => $this->masterParams($username), 'retryStrategy' => new MysqlRetryStrategy($n), ]; - $params['connectionParams']['user'] = $username; $this->connections[$connectionName] = [ 'params' => $params, 'instance' => null, @@ -275,20 +276,13 @@ public function aRetryConnectionLimitedToRetryWithUsername($connectionName, $n, } /** - * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry with db :db and username :username * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry with db :db + * @Given a retry master\/slaves connection :connectionName with :slaveCount slaves limited to :n retry with db :db and username :username */ - public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetryWithDb($connectionName, $slaveCount, $n, $db, $username = null, $password = '') + public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetryWithDbAndUsername($connectionName, $slaveCount, $n, $db, $username = null) { - $master = $this->masterParams(); + $master = $this->masterParams($username); $master['dbname'] = $db; - if ($username !== null) { - $master['user'] = $username; - if ($username === 'root') { - $password = getenv('DBLINKER_DB_1_ENV_MYSQL_ROOT_PASSWORD'); - } - $master['password'] = $password; - } $slaveCount = (int) $slaveCount; $slaves = []; @@ -315,19 +309,12 @@ public function aRetryMasterSlavesConnectionWithSlavesLimitedToRetryWithDb($conn } /** - * @Given a retry connection :connectionName limited to :n retry with db :db and username :username * @Given a retry connection :connectionName limited to :n retry with db :db + * @Given a retry connection :connectionName limited to :n retry with db :db and username :username */ - public function aRetryConnectionLimitedToRetryWithDb($connectionName, $n, $db, $username = null) + public function aRetryConnectionLimitedToRetryWithDbAndUsername($connectionName, $n, $db, $username = null) { - $master = $this->masterParams(); - if ($username !== null) { - $master['user'] = $username; - if ($username === 'root') { - $password = getenv('DBLINKER_DB_1_ENV_MYSQL_ROOT_PASSWORD'); - } - $master['password'] = $password; - } + $master = $this->masterParams($username); $params = [ 'driverClass' => 'Ez\DbLinker\Driver\MysqlRetryDriver', 'connectionParams' => $master, @@ -453,6 +440,20 @@ public function shouldHaveSlave($connectionName, $n) assert($slaveCount === (int)$n, "Slaves count is $slaveCount, $n expected."); } + /** + * @Given a connection :connectionName + * @Given a connection :connectionName with username :username + */ + public function aConnectionWithusername($connectionName, $username = null) + { + $this->connections[$connectionName] = [ + 'params' => $this->masterParams($username), + 'instance' => null, + 'last-result' => null, + 'last-error' => null, + ]; + } + private function getWrappedConnection($connectionName) { return $this->getConnection($connectionName)->getWrappedConnection(); @@ -468,7 +469,24 @@ private function getConnection($connectionName) return $this->connections[$connectionName]['instance']; } - abstract protected function masterParams(); + protected abstract function params(Array $params); + + private function masterParams($username = null, $password = '') { + $params = [ + 'host' => getenv('DBLINKER_DB_1_PORT_3306_TCP_ADDR'), + 'user' => getenv('DBLINKER_DB_1_ENV_MYSQL_USER'), + 'password' => getenv('DBLINKER_DB_1_ENV_MYSQL_PASSWORD'), + 'dbname' => getenv('DBLINKER_DB_1_ENV_MYSQL_DATABASE'), + ]; + if ($username !== null) { + $params['user'] = $username; + if ($username === 'root') { + $password = getenv('DBLINKER_DB_1_ENV_MYSQL_ROOT_PASSWORD'); + } + $params['password'] = $password; + } + return $this->params($params); + } } class MysqlRetryStrategy extends Ez\DbLinker\RetryStrategy\MysqlRetryStrategy @@ -478,12 +496,11 @@ class MysqlRetryStrategy extends Ez\DbLinker\RetryStrategy\MysqlRetryStrategy public function shouldRetry( Doctrine\DBAL\DBALException $exception, Ez\DbLinker\Driver\Connection\RetryConnection $connection, - Doctrine\DBAL\Driver\Connection $wrappedConnection, $method, Array $arguments ) { $this->lastError = $exception; - return parent::shouldRetry($exception, $connection, $wrappedConnection, $method, $arguments); + return parent::shouldRetry($exception, $connection, $method, $arguments); } public function lastError() diff --git a/features/bootstrap/MysqliContext.php b/features/bootstrap/MysqliContext.php index caf3e88..774c7a0 100644 --- a/features/bootstrap/MysqliContext.php +++ b/features/bootstrap/MysqliContext.php @@ -7,14 +7,9 @@ class MysqliContext implements Context, SnippetAcceptingContext { use FeatureContext; - private function masterParams() + private function params(Array $params) { - return [ - 'host' => getenv('DBLINKER_DB_1_PORT_3306_TCP_ADDR'), - 'user' => getenv('DBLINKER_DB_1_ENV_MYSQL_USER'), - 'password' => getenv('DBLINKER_DB_1_ENV_MYSQL_PASSWORD'), - 'dbname' => getenv('DBLINKER_DB_1_ENV_MYSQL_DATABASE'), - 'driver' => 'mysqli', - ]; + $params['driver'] = 'mysqli'; + return $params; } } diff --git a/features/bootstrap/PdoMysqlContext.php b/features/bootstrap/PdoMysqlContext.php index b317277..54e15dc 100644 --- a/features/bootstrap/PdoMysqlContext.php +++ b/features/bootstrap/PdoMysqlContext.php @@ -7,18 +7,13 @@ class PdoMysqlContext implements Context, SnippetAcceptingContext { use FeatureContext; - private function masterParams() + private function params(Array $params) { - return [ - 'host' => getenv('DBLINKER_DB_1_PORT_3306_TCP_ADDR'), - 'user' => getenv('DBLINKER_DB_1_ENV_MYSQL_USER'), - 'password' => getenv('DBLINKER_DB_1_ENV_MYSQL_PASSWORD'), - 'dbname' => getenv('DBLINKER_DB_1_ENV_MYSQL_DATABASE'), - 'driver' => 'pdo_mysql', - 'driverOptions' => [ - // todo move to retry driver - PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, - ], + $params['driver'] = 'pdo_mysql'; + $params['driverOptions'] = [ + // todo move to retry driver + PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, ]; + return $params; } } diff --git a/features/retry-master-slave.feature b/features/retry-master-slave.feature index 994b5a8..6957ea3 100644 --- a/features/retry-master-slave.feature +++ b/features/retry-master-slave.feature @@ -1,6 +1,6 @@ -@retry @master-slaves @wip +@retry @master-slaves Feature: Retry Master/Slaves - + @bug Scenario: ACCESS_DENIED_ERROR restart on another slave Given a retry master/slaves connection "conn" with 2 slaves limited to 1 retry with username "nobody" When I query "SELECT 1" on "conn" @@ -17,7 +17,7 @@ Feature: Retry Master/Slaves And the last error code should be 1045 on "conn" And "conn" retry limit should be 1 And "conn" should have 2 slaves - + @bug Scenario: ER_BAD_DB_ERROR restart on another slave Given a retry master/slaves connection "conn" with 2 slaves limited to 1 retry with db "unknown_db" and username "root" When I query "SELECT 1" on "conn" diff --git a/features/retry.feature b/features/retry.feature index de0233d..edb28f6 100644 --- a/features/retry.feature +++ b/features/retry.feature @@ -67,3 +67,13 @@ Feature: Retry And I query "SELECT 1" on "conn" Then the last query succeeded on "conn" And "conn" retry limit should be 0 +@wip + Scenario: Too many connections + Given the server accept 1 more connections + And a retry connection "conn1" limited to 1 retry + And a retry connection "conn2" limited to 1 retry + When I query "SELECT 1" on "conn1" + And I query "SELECT 1" on "conn2" + Then the last query failed on "conn2" + And the last error code should be 1040 on "conn2" + And "conn2" retry limit should be 0 diff --git a/src/Driver/Connection/RetryConnection.php b/src/Driver/Connection/RetryConnection.php index e2d4dde..f111722 100644 --- a/src/Driver/Connection/RetryConnection.php +++ b/src/Driver/Connection/RetryConnection.php @@ -147,7 +147,6 @@ private function callAndRetry($method, Array $arguments) if (!$this->retryStrategy->shouldRetry( $exception, $this, - $connection->getWrappedConnection(), $method, $arguments )) { diff --git a/src/RetryStrategy.php b/src/RetryStrategy.php index 224fa98..ec09461 100644 --- a/src/RetryStrategy.php +++ b/src/RetryStrategy.php @@ -11,7 +11,6 @@ interface RetryStrategy public function shouldRetry( DBALException $exception, RetryConnection $connection, - Connection $wrappedConnection, $method, Array $arguments ); diff --git a/src/RetryStrategy/MysqlRetryStrategy.php b/src/RetryStrategy/MysqlRetryStrategy.php index db44be4..d31ea59 100644 --- a/src/RetryStrategy/MysqlRetryStrategy.php +++ b/src/RetryStrategy/MysqlRetryStrategy.php @@ -36,6 +36,39 @@ public function __construct($retryLimit = INF) $this->retryLimit = $retryLimit; } + public function shouldRetry( + DBALException $exception, + RetryConnection $connection, + $method, + Array $arguments + ) { + if (!$this->canRetry($connection)) { + return false; + } + $strategy = $this->errorCodeStrategy($this->errorCode($exception)); + return $this->applyStrategy($strategy, $connection); + } + + public function retryLimit() + { + return $this->retryLimit > 0 ? (int) $this->retryLimit : 0; + } + + private function canRetry(RetryConnection $connection = null) + { + return $this->retryLimit > 0 && ($connection === null || $connection->transactionLevel() === 0); + } + + private function errorCode(DBALException $exception) + { + while ($exception !== null) { + if ($exception instanceof DriverException) { + return $exception->getErrorCode(); + } + $exception = $exception->getPrevious(); + } + } + private function errorCodeStrategy($errorCode) { $strategy = (object) [ @@ -53,23 +86,24 @@ private function errorCodeStrategy($errorCode) return (object) ['retry' => false]; } - private function errorCode(DBALException $exception) - { - while ($exception !== null) { - if ($exception instanceof DriverException) { - return $exception->getErrorCode(); - } - $exception = $exception->getPrevious(); + private function applyStrategy(stdClass $strategy, RetryConnection $connection) { + if ($strategy->retry === false || !$this->changeServer($strategy, $connection)) { + return false; } + sleep($strategy->wait); + $this->reconnect($strategy, $connection); + $this->retryLimit--; + return true; } - private function changeServer(stdClass $strategy, Connection $connection) + private function changeServer(stdClass $strategy, RetryConnection $connection) { if (!$strategy->changeServer) { return true; } - if ($connection instanceof MasterSlavesConnection && !$connection->isConnectedToMaster()) { - $connection->disableCurrentSlave(); + $wrappedConnection = $connection->wrappedConnection()->getWrappedConnection(); + if ($wrappedConnection instanceof MasterSlavesConnection && !$wrappedConnection->isConnectedToMaster()) { + $wrappedConnection->disableCurrentSlave(); return true; } return false; @@ -81,33 +115,4 @@ private function reconnect(stdClass $strategy, RetryConnection $connection) $connection->close(); } } - - private function applyStrategy(stdClass $strategy, RetryConnection $connection, Connection $wrappedConnection) { - if ($strategy->retry === false || !$this->changeServer($strategy, $wrappedConnection)) { - return false; - } - sleep($strategy->wait); - $this->reconnect($strategy, $connection); - $this->retryLimit--; - return true; - } - - public function shouldRetry( - DBALException $exception, - RetryConnection $connection, - Connection $wrappedConnection, - $method, - Array $arguments - ) { - if ($connection->transactionLevel() > 0 || $this->retryLimit < 1) { - return false; - } - $strategy = $this->errorCodeStrategy($this->errorCode($exception)); - return $this->applyStrategy($strategy, $connection, $wrappedConnection); - } - - public function retryLimit() - { - return $this->retryLimit > 0 ? (int) $this->retryLimit : 0; - } } From cc5b5307a29af1a69fa0211eb197f1e2542608bc Mon Sep 17 00:00:00 2001 From: Mathieu Rochette Date: Thu, 28 May 2015 13:40:04 +0200 Subject: [PATCH 2/2] handle "Too many connections" --- src/RetryStrategy/MysqlRetryStrategy.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/RetryStrategy/MysqlRetryStrategy.php b/src/RetryStrategy/MysqlRetryStrategy.php index d31ea59..1dc2915 100644 --- a/src/RetryStrategy/MysqlRetryStrategy.php +++ b/src/RetryStrategy/MysqlRetryStrategy.php @@ -15,6 +15,8 @@ class MysqlRetryStrategy implements RetryStrategy private $retryLimit; private $errorCodeStrategies = [ + // ER_CON_COUNT_ERROR + 1040 => ['wait' => 1], // ER_DBACCESS_DENIED_ERROR 1044 => ['changeServer' => true], // ER_ACCESS_DENIED_ERROR