Skip to content

Commit

Permalink
Retry fixes (#1815)
Browse files Browse the repository at this point in the history 
* fix retrying on message for guzzle request exceptions

* expose ability to configure backoff delay

* handle trait conflicts/correctly override backoff delay
  • Loading branch information
@dwsupplee
dwsupplee authored Apr 9, 2019
1 parent 660fca0 commit d2af56b
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 52 deletions.
8 changes: 5 additions & 3 deletions BigQuery/src/BigQueryClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@
class BigQueryClient
{
use ArrayTrait;
use ClientTrait;
use RetryDeciderTrait;
use ClientTrait, RetryDeciderTrait {
ClientTrait::jsonEncode insteadof RetryDeciderTrait;
ClientTrait::jsonDecode insteadof RetryDeciderTrait;
}

const VERSION = '1.5.0';

Expand Down Expand Up @@ -124,7 +126,7 @@ public function __construct(array $config = [])
'projectIdRequired' => true,
'returnInt64AsObject' => false,
'restRetryFunction' => $this->getRetryFunction(),
'restDelayFunction' => function ($attempt) {
'restCalcDelayFunction' => function ($attempt) {
return min(
mt_rand(0, 1000000) + (pow(2, $attempt) * 1000000),
self::MAX_DELAY_MICROSECONDS
Expand Down
24 changes: 22 additions & 2 deletions Core/src/ExponentialBackoff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ExponentialBackoff
private $retries;

/**
* @var callable
* @var callable|null
*/
private $retryFunction;

Expand All @@ -39,6 +39,11 @@ class ExponentialBackoff
*/
private $delayFunction;

/**
* @var callable|null
*/
private $calcDelayFunction;

/**
* @param int $retries [optional] Number of retries for a failed request.
* @param callable $retryFunction [optional] returns bool for whether or not to retry
Expand Down Expand Up @@ -66,6 +71,7 @@ public function __construct($retries = null, callable $retryFunction = null)
public function execute(callable $function, array $arguments = [])
{
$delayFunction = $this->delayFunction;
$calcDelayFunction = $this->calcDelayFunction ?: [$this, 'calculateDelay'];
$retryAttempt = 0;
$exception = null;

Expand All @@ -83,7 +89,7 @@ public function execute(callable $function, array $arguments = [])
break;
}

$delayFunction($this->calculateDelay($retryAttempt));
$delayFunction($calcDelayFunction($retryAttempt));
$retryAttempt++;
}
}
Expand All @@ -92,6 +98,8 @@ public function execute(callable $function, array $arguments = [])
}

/**
* If not set, defaults to using `usleep`.
*
* @param callable $delayFunction
* @return void
*/
Expand All @@ -100,6 +108,18 @@ public function setDelayFunction(callable $delayFunction)
$this->delayFunction = $delayFunction;
}

/**
* If not set, defaults to using
* {@see Google\Cloud\Core\ExponentialBackoff::calculateDelay()}.
*
* @param callable $calcDelayFunction
* @return void
*/
public function setCalcDelayFunction(callable $calcDelayFunction)
{
$this->calcDelayFunction = $calcDelayFunction;
}

/**
* Calculates exponential delay.
*
Expand Down
4 changes: 1 addition & 3 deletions Core/src/Logger/AppEngineFlexFormatter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
*/
class AppEngineFlexFormatter extends LineFormatter
{
use JsonTrait;

/**
* @param string $format [optional] The format of the message
* @param string $dateFormat [optional] The format of the timestamp
Expand Down Expand Up @@ -62,6 +60,6 @@ public function format(array $record)
$_SERVER['HTTP_X_CLOUD_TRACE_CONTEXT']
)[0];
}
return "\n" . $this->jsonEncode($payload);
return "\n" . json_encode($payload);
}
}
93 changes: 52 additions & 41 deletions Core/src/RequestWrapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
*/
class RequestWrapper
{
use JsonTrait;
use RequestWrapperTrait;
use RetryDeciderTrait;

Expand Down Expand Up @@ -82,16 +81,15 @@ class RequestWrapper
private $retryFunction;

/**
* @var callable|null Sets the conditions for determining how long to wait
* between attempts to retry.
* @var callable Executes a delay.
*/
private $restDelayFunction;
private $delayFunction;

/**
* @var callable Sets the conditions for determining how long to wait
* @var callable|null Sets the conditions for determining how long to wait
* between attempts to retry.
*/
private $delayFunction;
private $calcDelayFunction;

/**
* @param array $config [optional] {
Expand All @@ -115,9 +113,14 @@ class RequestWrapper
* @type array $restOptions HTTP client specific configuration options.
* @type bool $shouldSignRequest Whether to enable request signing.
* @type callable $restRetryFunction Sets the conditions for whether or
* not a request should attempt to retry.
* @type callable $restDelayFunction Sets the conditions for determining
* how long to wait between attempts to retry.
* not a request should attempt to retry. Function signature should
* match: `function (\Exception $ex) : bool`.
* @type callable $restDelayFunction Executes a delay, defaults to
* utilizing `usleep`. Function signature should match:
* `function (int $delay) : void`.
* @type callable $restCalcDelayFunction Sets the conditions for
* determining how long to wait between attempts to retry. Function
* signature should match: `function (int $attempt) : int`.
* }
*/
public function __construct(array $config = [])
Expand All @@ -132,7 +135,8 @@ public function __construct(array $config = [])
'shouldSignRequest' => true,
'componentVersion' => null,
'restRetryFunction' => null,
'restDelayFunction' => null
'restDelayFunction' => null,
'restCalcDelayFunction' => null
];

$this->componentVersion = $config['componentVersion'];
Expand All @@ -143,6 +147,7 @@ public function __construct(array $config = [])
$this->delayFunction = $config['restDelayFunction'] ?: function ($delay) {
usleep($delay);
};
$this->calcDelayFunction = $config['restCalcDelayFunction'];
$this->httpHandler = $config['httpHandler'] ?: HttpHandlerFactory::build();
$this->authHttpHandler = $config['authHttpHandler'] ?: $this->httpHandler;
$this->asyncHttpHandler = $config['asyncHttpHandler'] ?: $this->buildDefaultAsyncHandler();
Expand All @@ -164,22 +169,34 @@ public function __construct(array $config = [])
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type callable $restRetryFunction Sets the conditions for whether or
* not a request should attempt to retry.
* @type callable $restDelayFunction Sets the conditions for determining
* how long to wait between attempts to retry.
* not a request should attempt to retry. Function signature should
* match: `function (\Exception $ex) : bool`.
* @type callable $restDelayFunction Executes a delay, defaults to
* utilizing `usleep`. Function signature should match:
* `function (int $delay) : void`.
* @type callable $restCalcDelayFunction Sets the conditions for
* determining how long to wait between attempts to retry. Function
* signature should match: `function (int $attempt) : int`.
* @type array $restOptions HTTP client specific configuration options.
* }
* @return ResponseInterface
*/
public function send(RequestInterface $request, array $options = [])
{
$retryOptions = $this->getRetryOptions($options);
$backoff = $this->configureBackoff(
$backoff = new ExponentialBackoff(
$retryOptions['retries'],
$retryOptions['retryFunction'],
$retryOptions['delayFunction']
$retryOptions['retryFunction']
);

if ($retryOptions['delayFunction']) {
$backoff->setDelayFunction($retryOptions['delayFunction']);
}

if ($retryOptions['calcDelayFunction']) {
$backoff->setCalcDelayFunction($retryOptions['calcDelayFunction']);
}

try {
return $backoff->execute($this->httpHandler, [
$this->applyHeaders($request),
Expand All @@ -202,9 +219,14 @@ public function send(RequestInterface $request, array $options = [])
* @type int $retries Number of retries for a failed request.
* **Defaults to** `3`.
* @type callable $restRetryFunction Sets the conditions for whether or
* not a request should attempt to retry.
* @type callable $restDelayFunction Sets the conditions for determining
* how long to wait between attempts to retry.
* not a request should attempt to retry. Function signature should
* match: `function (\Exception $ex) : bool`.
* @type callable $restDelayFunction Executes a delay, defaults to
* utilizing `usleep`. Function signature should match:
* `function (int $delay) : void`.
* @type callable $restCalcDelayFunction Sets the conditions for
* determining how long to wait between attempts to retry. Function
* signature should match: `function (int $attempt) : int`.
* @type array $restOptions HTTP client specific configuration options.
* }
* @return PromiseInterface<ResponseInterface>
Expand All @@ -221,6 +243,9 @@ public function sendAsync(RequestInterface $request, array $options = [])
$fn = function ($retryAttempt) use (&$fn, $request, $options) {
$asyncHttpHandler = $this->asyncHttpHandler;
$retryOptions = $this->getRetryOptions($options);
if (!$retryOptions['calcDelayFunction']) {
$retryOptions['calcDelayFunction'] = [ExponentialBackoff::class, 'calculateDelay'];
}

return $asyncHttpHandler(
$this->applyHeaders($request),
Expand All @@ -232,7 +257,8 @@ public function sendAsync(RequestInterface $request, array $options = [])
throw $this->convertToGoogleException($ex);
}

$retryOptions['delayFunction'](ExponentialBackoff::calculateDelay($retryAttempt));
$delay = $retryOptions['calcDelayFunction']($retryAttempt);
$retryOptions['delayFunction']($delay);
$retryAttempt++;

return $fn($retryAttempt);
Expand Down Expand Up @@ -339,13 +365,14 @@ private function convertToGoogleException(\Exception $ex)
/**
* Gets the exception message.
*
* @access private
* @param \Exception $ex
* @return string
*/
private function getExceptionMessage(\Exception $ex)
{
if ($ex instanceof RequestException && $ex->hasResponse()) {
$res = (string) $ex->getResponse()->getBody();
return (string) $ex->getResponse()->getBody();

try {
$this->jsonDecode($res);
Expand All @@ -358,25 +385,6 @@ private function getExceptionMessage(\Exception $ex)
return $ex->getMessage();
}

/**
* Configures an exponential backoff implementation.
*
* @param int $retries
* @param callable $retryFunction
* @param callable $delayFunction
* @return ExponentialBackoff
*/
private function configureBackoff($retries, callable $retryFunction, callable $delayFunction)
{
$backoff = new ExponentialBackoff($retries, $retryFunction);

if ($delayFunction) {
$backoff->setDelayFunction($delayFunction);
}

return $backoff;
}

/**
* Gets a set of request options.
*
Expand Down Expand Up @@ -416,7 +424,10 @@ private function getRetryOptions(array $options)
: $this->retryFunction,
'delayFunction' => isset($options['restDelayFunction'])
? $options['restDelayFunction']
: $this->delayFunction
: $this->delayFunction,
'calcDelayFunction' => isset($options['restCalcDelayFunction'])
? $options['restCalcDelayFunction']
: $this->calcDelayFunction
];
}

Expand Down
17 changes: 16 additions & 1 deletion Core/src/RetryDeciderTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@

namespace Google\Cloud\Core;

use GuzzleHttp\Exception\RequestException;

/**
* Provides methods for deciding if a request should be retried.
*/
trait RetryDeciderTrait
{
use JsonTrait;

/**
* @var array
*/
Expand Down Expand Up @@ -62,7 +66,18 @@ private function getRetryFunction($shouldRetryMessages = true)
return false;
}

$message = json_decode($ex->getMessage(), true);
$message = ($ex instanceof RequestException && $ex->hasResponse())
? (string) $ex->getResponse()->getBody()
: $ex->getMessage();

try {
$message = $this->jsonDecode(
$message,
true
);
} catch (\InvalidArgumentException $ex) {
return false;
}

if (!isset($message['error']['errors'])) {
return false;
Expand Down
25 changes: 25 additions & 0 deletions Core/tests/Unit/ExponentialBackoffTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,31 @@ public function testSuccessWithNoRetries()
$this->assertEquals(1, $actualAttempts);
}

public function testSetsCalculateDelayFunction()
{
$backoff = new ExponentialBackoff();
$hasTriggeredException = false;
$actualDelayAmount = 0;
$expectedDelayAmount = 100;
$backoff->setDelayFunction(function ($delay) use (&$actualDelayAmount) {
$actualDelayAmount = $delay;
});
$backoff->setCalcDelayFunction(function () use ($expectedDelayAmount) {
return $expectedDelayAmount;
});

try {
$backoff->execute(function () {
throw new \Exception();
});
} catch (\Exception $ex) {
$hasTriggeredException = true;
}

$this->assertTrue($hasTriggeredException);
$this->assertEquals($expectedDelayAmount, $actualDelayAmount);
}

/**
* @dataProvider delayProvider
*/
Expand Down
Loading

0 comments on commit d2af56b

Please sign in to comment.