Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry fixes #1815

Merged
merged 3 commits into from
Apr 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions BigQuery/src/BigQueryClient.php
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
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
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
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
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
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