diff --git a/README.md b/README.md index 808f976..c628ec8 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ This library is intended to make it easier for you to get started with and to us ## Quick Start ## ```bash -composer require "tomwalder/php-gds:^5.1" +composer require "tomwalder/php-gds:^6.1" ``` ```php // Build a new entity @@ -28,6 +28,18 @@ foreach($obj_store->fetchAll() as $obj_book) { } ``` +## New in Version 6.1 ## + +Support for automated exponential backoff for some types of errors. See documentation here: +https://cloud.google.com/datastore/docs/concepts/errors + +To enable: +```php +\GDS\Gateway::exponentialBackoff(true); +``` + +Version `6.0.0` introduced better (but different) support for `NULL` values. + ## New in Version 5.0 ## **As of version 5 (May 2021), this library provides support for** @@ -523,6 +535,10 @@ A full suite of unit tests is in the works. Assuming you've installed `php-gds` ```bash vendor/bin/phpunit ``` +Or, if you need to run containerised tests, you can use the `runphp` image (or any you choose) +```bash +docker run --rm -it -v`pwd`:/app -w /app fluentthinking/runphp:7.4.33-v0.9.0 php /app/vendor/bin/phpunit +``` [Click here for more details](tests/). diff --git a/src/GDS/Gateway.php b/src/GDS/Gateway.php index 3d4be83..d3e43fa 100644 --- a/src/GDS/Gateway.php +++ b/src/GDS/Gateway.php @@ -29,6 +29,8 @@ */ abstract class Gateway { + // 8 = about 5 seconds total, with last gap ~2.5 seconds + const RETRY_MAX_ATTEMPTS = 8; /** * The dataset ID @@ -72,6 +74,19 @@ abstract class Gateway */ protected $arr_kind_mappers = []; + protected static $bol_retry = false; + + /** + * Configure gateway retries (for 503, 500 responses) + * + * @param bool $bol_retry + * @return void + */ + public static function exponentialBackoff(bool $bol_retry = true) + { + self::$bol_retry = $bol_retry; + } + /** * Set the Schema to be used next (once?) * @@ -335,6 +350,66 @@ protected function configureValueParamForQuery($obj_val, $mix_value) return $obj_val; } + /** + * Delay execution, based on the attempt number + * + * @param int $int_attempt + * @return void + */ + protected function backoff(int $int_attempt) + { + $int_backoff = (int) pow(2, $int_attempt); + $int_jitter = rand(0, 10) * 1000; + $int_delay = ($int_backoff * 10000) + $int_jitter; + usleep($int_delay); + } + + /** + * Execute the callback with exponential backoff + * + * @param callable $fnc_main + * @param string|null $str_exception + * @param callable|null $fnc_resolve_exception + * @return mixed + * @throws \Throwable + */ + protected function executeWithExponentialBackoff( + callable $fnc_main, + string $str_exception = null, + callable $fnc_resolve_exception = null + ) { + $int_attempt = 0; + $bol_retry_once = false; + do { + try { + $int_attempt++; + if ($int_attempt > 1) { + $this->backoff($int_attempt); + } + return $fnc_main(); + } catch (\Throwable $obj_thrown) { + // Rethrow if we're not interested in this Exception type + if (null !== $str_exception && !$obj_thrown instanceof $str_exception) { + throw $obj_thrown; + } + // Rethrow if retry is disabled, non-retryable errors, or if we have hit a retry limit + if (false === self::$bol_retry || + true === $bol_retry_once || + !in_array((int) $obj_thrown->getCode(), static::RETRY_ERROR_CODES) + ) { + throw null === $fnc_resolve_exception ? $obj_thrown : $fnc_resolve_exception($obj_thrown); + } + // Just one retry for some errors + if (in_array((int) $obj_thrown->getCode(), static::RETRY_ONCE_CODES)) { + $bol_retry_once = true; + } + } + } while ($int_attempt < self::RETRY_MAX_ATTEMPTS); + + // We could not make this work after max retries + throw null === $fnc_resolve_exception ? $obj_thrown : $fnc_resolve_exception($obj_thrown); + } + /** * Configure a Value parameter, based on the supplied object-type value * diff --git a/src/GDS/Gateway/GRPCv1.php b/src/GDS/Gateway/GRPCv1.php index e6e6fd0..5a07dee 100755 --- a/src/GDS/Gateway/GRPCv1.php +++ b/src/GDS/Gateway/GRPCv1.php @@ -35,6 +35,7 @@ use Google\Cloud\Datastore\V1\GqlQuery; use Google\Cloud\Datastore\V1\GqlQueryParameter; use Google\Cloud\Datastore\V1\Value; +use Google\Rpc\Code; /** * gRPC Datastore Gateway (v1) @@ -46,6 +47,20 @@ */ class GRPCv1 extends \GDS\Gateway { + // https://cloud.google.com/datastore/docs/concepts/errors + // https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto + const RETRY_ERROR_CODES = [ + Code::UNKNOWN, + Code::ABORTED, + Code::DEADLINE_EXCEEDED, + Code::RESOURCE_EXHAUSTED, + Code::UNAVAILABLE, + Code::INTERNAL, + ]; + + const RETRY_ONCE_CODES = [ + Code::INTERNAL, + ]; /** * Cloud Datastore (gRPC & REST) Client @@ -98,29 +113,44 @@ private function createPartitionId() /** * Execute a method against the Datastore client. * + * Prepend projectId as first parameter automatically. + * * @param string $str_method * @param mixed[] $args * @return mixed * @throws \Exception */ - private function execute($str_method, $args) + private function execute(string $str_method, array $args) { + array_unshift($args, $this->str_dataset_id); + return $this->executeWithExponentialBackoff( + function () use ($str_method, $args) { + $this->obj_last_response = call_user_func_array([self::$obj_datastore_client, $str_method], $args); + return $this->obj_last_response; + }, + ApiException::class, + [$this, 'resolveExecuteException'] + ); + } + + /** + * Wrap the somewhat murky ApiException into something more useful + * + * https://cloud.google.com/datastore/docs/concepts/errors + * + * @param ApiException $obj_exception + * @return \Exception + */ + protected function resolveExecuteException(ApiException $obj_exception): \Exception { - try { - // Call gRPC client, - // prepend projectId as first parameter automatically. - array_unshift($args, $this->str_dataset_id); - $this->obj_last_response = call_user_func_array([self::$obj_datastore_client, $str_method], $args); - } catch (ApiException $obj_exception) { - $this->obj_last_response = null; - if (FALSE !== strpos($obj_exception->getMessage(), 'too much contention') || FALSE !== strpos($obj_exception->getMessage(), 'Concurrency')) { - // LIVE: "too much contention on these datastore entities. please try again." LOCAL : "Concurrency exception." - throw new Contention('Datastore contention', 409, $obj_exception); - } else { - throw $obj_exception; - } + $this->obj_last_response = null; + if (Code::ABORTED === $obj_exception->getCode() || + false !== strpos($obj_exception->getMessage(), 'too much contention') || + false !== strpos($obj_exception->getMessage(), 'Concurrency')) { + // LIVE: "too much contention on these datastore entities. please try again." + // LOCAL : "Concurrency exception." + return new Contention('Datastore contention', 409, $obj_exception); } - - return $this->obj_last_response; + return $obj_exception; } /** diff --git a/src/GDS/Gateway/RESTv1.php b/src/GDS/Gateway/RESTv1.php index 4582a38..281522e 100644 --- a/src/GDS/Gateway/RESTv1.php +++ b/src/GDS/Gateway/RESTv1.php @@ -5,6 +5,7 @@ use GuzzleHttp\Client; use GuzzleHttp\ClientInterface; use GuzzleHttp\HandlerStack; +use Psr\Http\Message\ResponseInterface; /** * Gateway, implementing the Datastore API v1 over REST @@ -23,6 +24,12 @@ class RESTv1 extends \GDS\Gateway const MODE_NON_TRANSACTIONAL = 'NON_TRANSACTIONAL'; const MODE_UNSPECIFIED = 'UNSPECIFIED'; + // https://cloud.google.com/datastore/docs/concepts/errors + // https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto + const RETRY_ERROR_CODES = [409, 429, 500, 503, 504]; + + const RETRY_ONCE_CODES = [500]; + /** * Client config keys. */ @@ -165,8 +172,14 @@ private function executePostRequest($str_action, $obj_request_body = null) if(null !== $obj_request_body) { $arr_options['json'] = $obj_request_body; } - $obj_response = $this->httpClient()->post($this->actionUrl($str_action), $arr_options); - $this->obj_last_response = json_decode((string)$obj_response->getBody()); + $str_url = $this->actionUrl($str_action); + $obj_response = $this->executeWithExponentialBackoff( + function () use ($str_url, $arr_options) { + return $this->httpClient()->post($str_url, $arr_options); + }, + \GuzzleHttp\Exception\RequestException::class + ); + $this->obj_last_response = \json_decode((string)$obj_response->getBody()); } /** @@ -492,7 +505,7 @@ public function beginTransaction($bol_cross_group = FALSE) * @return string */ protected function getBaseUrl() { - $str_base_url = $this->obj_http_client->getConfig(self::CONFIG_CLIENT_BASE_URL); + $str_base_url = $this->httpClient()->getConfig(self::CONFIG_CLIENT_BASE_URL); if (!empty($str_base_url)) { return $str_base_url; } diff --git a/tests/BackoffTest.php b/tests/BackoffTest.php new file mode 100644 index 0000000..50b6670 --- /dev/null +++ b/tests/BackoffTest.php @@ -0,0 +1,134 @@ + + */ +class BackoffTest extends \PHPUnit\Framework\TestCase +{ + public function testOnceAndReturn() + { + \GDS\Gateway::exponentialBackoff(true); + $shouldBeCalled = $this->getMockBuilder(\stdClass::class) + ->addMethods(['__invoke']) + ->getMock(); + $shouldBeCalled->expects($this->once()) + ->method('__invoke') + ->willReturn(87); + $int_result = $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + $this->assertEquals(87, $int_result); + } + + public function testBackoffCount() + { + \GDS\Gateway::exponentialBackoff(true); + $shouldBeCalled = $this->getMockBuilder(\stdClass::class) + ->addMethods(['__invoke']) + ->getMock(); + $shouldBeCalled->expects($this->exactly(\GDS\Gateway::RETRY_MAX_ATTEMPTS)) + ->method('__invoke') + ->willThrowException(new \RuntimeException('Test Exception', 503)); + $this->expectException(\RuntimeException::class); + $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + } + + public function testBackoffCountDisabled() + { + \GDS\Gateway::exponentialBackoff(false); + $shouldBeCalled = $this->getMockBuilder(\stdClass::class) + ->addMethods(['__invoke']) + ->getMock(); + $shouldBeCalled->expects($this->once()) + ->method('__invoke') + ->willThrowException(new \RuntimeException('Not retried', 503)); + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Not retried'); + $this->expectExceptionCode(503); + $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + } + + public function testPartialBackoff() { + \GDS\Gateway::exponentialBackoff(true); + $int_calls = 0; + $shouldBeCalled = function () use (&$int_calls) { + $int_calls++; + if ($int_calls < 4) { + throw new \RuntimeException('Always caught', 503); + } + return 42; + }; + $int_result = $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + $this->assertEquals(42, $int_result); + $this->assertEquals(4, $int_calls); + } + + + public function testIgnoredExceptionClass() + { + \GDS\Gateway::exponentialBackoff(true); + $shouldBeCalled = $this->getMockBuilder(\stdClass::class) + ->addMethods(['__invoke']) + ->getMock(); + $shouldBeCalled->expects($this->once()) + ->method('__invoke') + ->willThrowException(new \LogicException('Ignored', 503)); + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Ignored'); + $this->expectExceptionCode(503); + $this->buildTestGateway()->runExecuteWithExponentialBackoff( + $shouldBeCalled, + \RuntimeException::class + ); + } + + public function testIgnoredExceptionCode() + { + \GDS\Gateway::exponentialBackoff(true); + $shouldBeCalled = $this->getMockBuilder(\stdClass::class) + ->addMethods(['__invoke']) + ->getMock(); + $shouldBeCalled->expects($this->once()) + ->method('__invoke') + ->willThrowException(new \RuntimeException('Non-retry code', 42)); + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Non-retry code'); + $this->expectExceptionCode(42); + $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + } + + public function testRetryOnce() + { + \GDS\Gateway::exponentialBackoff(true); + $int_calls = 0; + $shouldBeCalled = function () use (&$int_calls) { + $int_calls++; + throw new \RuntimeException('Once', 500); + }; + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Once'); + $this->expectExceptionCode(500); + $this->buildTestGateway()->runExecuteWithExponentialBackoff($shouldBeCalled); + $this->assertEquals(2, $int_calls); + } + + private function buildTestGateway(): \RESTv1GatewayBackoff + { + return new RESTv1GatewayBackoff('dataset-id', 'my-app'); + } +} diff --git a/tests/base/RESTv1GatewayBackoff.php b/tests/base/RESTv1GatewayBackoff.php new file mode 100644 index 0000000..7ee7ea2 --- /dev/null +++ b/tests/base/RESTv1GatewayBackoff.php @@ -0,0 +1,13 @@ +executeWithExponentialBackoff($fnc_main, $str_exception, $fnc_resolve_exception); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 181451e..14bdd6b 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -12,7 +12,8 @@ require_once(dirname(__FILE__) . '/../vendor/autoload.php'); // Base Test Files +require_once(dirname(__FILE__) . '/base/RESTv1GatewayBackoff.php'); require_once(dirname(__FILE__) . '/base/RESTv1Test.php'); require_once(dirname(__FILE__) . '/base/Simple.php'); require_once(dirname(__FILE__) . '/base/Book.php'); -require_once(dirname(__FILE__) . '/base/FakeGuzzleClient.php'); \ No newline at end of file +require_once(dirname(__FILE__) . '/base/FakeGuzzleClient.php');