ソースを参照

Added connection retry logic to Driver. Configured SqlServer to retry connection on Azure Database paused error code.

Corey Taylor 5 年 前
コミット
046fb3576a

+ 29 - 18
src/Core/Retry/CommandRetry.php

@@ -34,22 +34,25 @@ class CommandRetry
     protected $strategy;
 
     /**
-     * The number of retries to perform in case of failure.
-     *
      * @var int
      */
-    protected $retries;
+    protected $maxRetries;
+
+    /**
+     * @var int
+     */
+    protected $numRetries;
 
     /**
      * Creates the CommandRetry object with the given strategy and retry count
      *
      * @param \Cake\Core\Retry\RetryStrategyInterface $strategy The strategy to follow should the action fail
-     * @param int $retries The number of times the action has been already called
+     * @param int $maxRetries The maximum number of retry attempts allowed
      */
-    public function __construct(RetryStrategyInterface $strategy, int $retries = 1)
+    public function __construct(RetryStrategyInterface $strategy, int $maxRetries = 1)
     {
         $this->strategy = $strategy;
-        $this->retries = $retries;
+        $this->maxRetries = $maxRetries;
     }
 
     /**
@@ -61,23 +64,31 @@ class CommandRetry
      */
     public function run(callable $action)
     {
-        $retryCount = 0;
-        $lastException = null;
-
-        do {
+        $this->numRetries = 0;
+        while (true) {
             try {
                 return $action();
             } catch (Exception $e) {
-                $lastException = $e;
-                if (!$this->strategy->shouldRetry($e, $retryCount)) {
-                    throw $e;
+                if (
+                    $this->numRetries < $this->maxRetries &&
+                    $this->strategy->shouldRetry($e, $this->numRetries)
+                ) {
+                    $this->numRetries++;
+                    continue;
                 }
-            }
-        } while ($this->retries > $retryCount++);
 
-        /** @psalm-suppress RedundantCondition */
-        if ($lastException !== null) {
-            throw $lastException;
+                throw $e;
+            }
         }
     }
+
+    /**
+     * Returns the last number of retry attemps.
+     *
+     * @return int
+     */
+    public function getRetries(): int
+    {
+        return $this->numRetries;
+    }
 }

+ 1 - 1
src/Core/Retry/RetryStrategyInterface.php

@@ -28,7 +28,7 @@ interface RetryStrategyInterface
      * Returns true if the action can be retried, false otherwise.
      *
      * @param \Exception $exception The exception that caused the action to fail
-     * @param int $retryCount The number of times the action has been already called
+     * @param int $retryCount The number of times action has been retried
      * @return bool Whether or not it is OK to retry the action
      */
     public function shouldRetry(Exception $exception, int $retryCount): bool;

+ 34 - 4
src/Database/Driver.php

@@ -17,7 +17,9 @@ declare(strict_types=1);
 namespace Cake\Database;
 
 use Cake\Core\App;
+use Cake\Core\Retry\CommandRetry;
 use Cake\Database\Exception\MissingConnectionException;
+use Cake\Database\Retry\ErrorCodeWaitStrategy;
 use Cake\Database\Schema\SchemaDialect;
 use Cake\Database\Schema\TableSchema;
 use Cake\Database\Statement\PDOStatement;
@@ -38,6 +40,11 @@ abstract class Driver implements DriverInterface
     protected const MAX_ALIAS_LENGTH = null;
 
     /**
+     * @var int[] DB-specific error codes that allow connect retry
+     */
+    protected const RETRY_ERROR_CODES = [];
+
+    /**
      * Instance of PDO.
      *
      * @var \PDO
@@ -82,6 +89,13 @@ abstract class Driver implements DriverInterface
     protected $_version;
 
     /**
+     * The last number of connection retry attempts.
+     *
+     * @var int
+     */
+    protected $connectRetries = 0;
+
+    /**
      * Constructor
      *
      * @param array $config The configuration for the driver.
@@ -110,13 +124,18 @@ abstract class Driver implements DriverInterface
      */
     protected function _connect(string $dsn, array $config): bool
     {
-        try {
-            $connection = new PDO(
+        $action = function () use ($dsn, $config) {
+            $this->setConnection(new PDO(
                 $dsn,
                 $config['username'] ?: null,
                 $config['password'] ?: null,
                 $config['flags']
-            );
+            ));
+        };
+
+        $retry = new CommandRetry(new ErrorCodeWaitStrategy(static::RETRY_ERROR_CODES, 5), 4);
+        try {
+            $retry->run($action);
         } catch (PDOException $e) {
             throw new MissingConnectionException(
                 [
@@ -126,8 +145,9 @@ abstract class Driver implements DriverInterface
                 null,
                 $e
             );
+        } finally {
+            $this->connectRetries = $retry->getRetries();
         }
-        $this->setConnection($connection);
 
         return true;
     }
@@ -487,6 +507,16 @@ abstract class Driver implements DriverInterface
     }
 
     /**
+     * Returns the number of connection retry attempts made.
+     *
+     * @return int
+     */
+    public function getConnectRetries(): int
+    {
+        return $this->connectRetries;
+    }
+
+    /**
      * Destructor
      */
     public function __destruct()

+ 7 - 0
src/Database/Driver/Sqlserver.php

@@ -47,6 +47,13 @@ class Sqlserver extends Driver
     protected const MAX_ALIAS_LENGTH = 128;
 
     /**
+     * @inheritDoc
+     */
+    protected const RETRY_ERROR_CODES = [
+        40613, // Azure Sql Database paused
+    ];
+
+    /**
      * Base configuration settings for Sqlserver driver
      *
      * @var array

+ 1 - 0
src/Database/DriverInterface.php

@@ -24,6 +24,7 @@ use Closure;
  * Interface for database driver.
  *
  * @method int|null getMaxAliasLength() Returns the maximum alias length allowed.
+ * @method int getConnectRetries() Returns the number of connection retry attempts made.
  */
 interface DriverInterface
 {

+ 69 - 0
src/Database/Retry/ErrorCodeWaitStrategy.php

@@ -0,0 +1,69 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         4.2.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Database\Retry;
+
+use Cake\Core\Retry\RetryStrategyInterface;
+use Exception;
+use PDOException;
+
+/**
+ * Implements retry strategy based on db error codes and wait interval.
+ *
+ * @internal
+ */
+class ErrorCodeWaitStrategy implements RetryStrategyInterface
+{
+    /**
+     * @var int[]
+     */
+    protected $errorCodes;
+
+    /**
+     * @var int
+     */
+    protected $retryInterval;
+
+    /**
+     * @param int[] $errorCodes DB-specific error codes that allow retrying
+     * @param int $retryInterval Seconds to wait before allowing next retry, 0 for no wait.
+     */
+    public function __construct(array $errorCodes, int $retryInterval)
+    {
+        $this->errorCodes = $errorCodes;
+        $this->retryInterval = $retryInterval;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function shouldRetry(Exception $exception, int $retryCount): bool
+    {
+        if (
+            $exception instanceof PDOException &&
+            $exception->errorInfo &&
+            in_array($exception->errorInfo[1], $this->errorCodes)
+        ) {
+            if ($this->retryInterval > 0) {
+                sleep($this->retryInterval);
+            }
+
+            return true;
+        }
+
+        return false;
+    }
+}

+ 2 - 4
src/Database/Retry/ReconnectStrategy.php

@@ -72,12 +72,10 @@ class ReconnectStrategy implements RetryStrategyInterface
     }
 
     /**
+     * {@inheritDoc}
+     *
      * 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, int $retryCount): bool
     {

+ 21 - 42
tests/TestCase/Core/Retry/CommandRetryTest.php

@@ -16,7 +16,6 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Core\Retry;
 
 use Cake\Core\Retry\CommandRetry;
-use Cake\Core\Retry\RetryStrategyInterface;
 use Cake\TestSuite\TestCase;
 use Exception;
 
@@ -33,30 +32,18 @@ class CommandRetryTest extends TestCase
     public function testRetry()
     {
         $count = 0;
-        $exception = new Exception('this is failing');
-        $action = function () use (&$count, $exception) {
-            $count++;
-
-            if ($count < 4) {
-                throw $exception;
+        $action = function () use (&$count) {
+            if ($count < 2) {
+                ++$count;
+                throw new Exception('this is failing');
             }
 
             return $count;
         };
 
-        $strategy = $this->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 + 1, $count);
-
-                return true;
-            }));
-
-        $retry = new CommandRetry($strategy, 5);
-        $this->assertEquals(4, $retry->run($action));
+        $strategy = new \TestApp\Database\Retry\TestRetryStrategy(true);
+        $retry = new CommandRetry($strategy, 2);
+        $this->assertEquals(2, $retry->run($action));
     }
 
     /**
@@ -66,20 +53,18 @@ class CommandRetryTest extends TestCase
      */
     public function testExceedAttempts()
     {
-        $exception = new Exception('this is failing');
-        $action = function () use ($exception) {
-            throw $exception;
-        };
+        $count = 0;
+        $action = function () use (&$count) {
+            if ($count < 2) {
+                ++$count;
+                throw new Exception('this is failing');
+            }
 
-        $strategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock();
-        $strategy
-            ->expects($this->exactly(4))
-            ->method('shouldRetry')
-            ->will($this->returnCallback(function ($e) {
-                return true;
-            }));
+            return $count;
+        };
 
-        $retry = new CommandRetry($strategy, 3);
+        $strategy = new \TestApp\Database\Retry\TestRetryStrategy(true);
+        $retry = new CommandRetry($strategy, 1);
         $this->expectException(Exception::class);
         $this->expectExceptionMessage('this is failing');
         $retry->run($action);
@@ -92,19 +77,13 @@ class CommandRetryTest extends TestCase
      */
     public function testRespectStrategy()
     {
-        $action = function () {
+        $count = 0;
+        $action = function () use (&$count) {
             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);
+        $strategy = new \TestApp\Database\Retry\TestRetryStrategy(false);
+        $retry = new CommandRetry($strategy, 2);
         $this->expectException(Exception::class);
         $this->expectExceptionMessage('this is failing');
         $retry->run($action);

+ 15 - 0
tests/TestCase/Database/ConnectionTest.php

@@ -228,6 +228,21 @@ class ConnectionTest extends TestCase
         $this->assertInstanceOf('PDOException', $e->getPrevious());
     }
 
+    public function testConnectRetry()
+    {
+        $this->skipIf(!ConnectionManager::get('test')->getDriver() instanceof \Cake\Database\Driver\Sqlserver);
+
+        $connection = new Connection(['driver' => 'RetryDriver']);
+        $this->assertInstanceOf('TestApp\Database\Driver\RetryDriver', $connection->getDriver());
+
+        try {
+            $connection->connect();
+        } catch (MissingConnectionException $e) {
+        }
+
+        $this->assertSame(4, $connection->getDriver()->getConnectRetries());
+    }
+
     /**
      * Tests creation of prepared statements
      *

+ 46 - 0
tests/test_app/TestApp/Database/Driver/RetryDriver.php

@@ -0,0 +1,46 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         4.2.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace TestApp\Database\Driver;
+
+use Cake\Database\Driver\Sqlserver;
+use Cake\Datasource\ConnectionManager;
+
+class RetryDriver extends Sqlserver
+{
+    /**
+     * @inheritDoc
+     */
+    protected const RETRY_ERROR_CODES = [18456];
+
+    /**
+     * @inheritDoc
+     */
+    public function connect(): bool
+    {
+        $testConfig = ConnectionManager::get('test')->config() + $this->_baseConfig;
+        $dsn = "sqlsrv:Server={$testConfig['host']};Database={$testConfig['database']}";
+
+        $this->_connect($dsn, ['username' => 'invalid', 'password' => '', 'flags' => []]);
+
+        return true;
+    }
+
+    public function enabled(): bool
+    {
+        return true;
+    }
+}

+ 41 - 0
tests/test_app/TestApp/Database/Retry/TestRetryStrategy.php

@@ -0,0 +1,41 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         4.2.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace TestApp\Database\Retry;
+
+use Cake\Core\Retry\RetryStrategyInterface;
+use Exception;
+
+class TestRetryStrategy implements RetryStrategyInterface
+{
+    /**
+     * @var bool
+     */
+    protected $allowRetry;
+
+    public function __construct(bool $allowRetry)
+    {
+        $this->allowRetry = $allowRetry;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function shouldRetry(Exception $exception, int $retryCount): bool
+    {
+        return $this->allowRetry;
+    }
+}