Skip to content

Commit

Permalink
feature #38532 [HttpClient] simplify retry mechanism around RetryStra…
Browse files Browse the repository at this point in the history
…tegyInterface (nicolas-grekas)

This PR was merged into the 5.x branch.

Discussion
----------

[HttpClient] simplify retry mechanism around RetryStrategyInterface

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Replaces #38466

I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons.

I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface.

Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that.

The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc).

/cc @jderusse

Commits
-------

544c3e8 [HttpClient] simplify retry mechanism around RetryStrategyInterface
  • Loading branch information
nicolas-grekas committed Oct 13, 2020
2 parents ee3294e + 544c3e8 commit f52f090
Show file tree
Hide file tree
Showing 17 changed files with 198 additions and 281 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1641,19 +1641,15 @@ private function addHttpClientRetrySection()
->addDefaultsIfNotSet()
->beforeNormalization()
->always(function ($v) {
if (isset($v['backoff_service']) && (isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
throw new \InvalidArgumentException('The "backoff_service" option cannot be used along with the "delay", "multiplier", "max_delay" or "jitter" options.');
}
if (isset($v['decider_service']) && (isset($v['http_codes']))) {
throw new \InvalidArgumentException('The "decider_service" option cannot be used along with the "http_codes" options.');
if (isset($v['retry_strategy']) && (isset($v['http_codes']) || isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
throw new \InvalidArgumentException('The "retry_strategy" option cannot be used along with the "http_codes", "delay", "multiplier", "max_delay" or "jitter" options.');
}

return $v;
})
->end()
->children()
->scalarNode('backoff_service')->defaultNull()->info('service id to override the retry backoff')->end()
->scalarNode('decider_service')->defaultNull()->info('service id to override the retry decider')->end()
->scalarNode('retry_strategy')->defaultNull()->info('service id to override the retry strategy')->end()
->arrayNode('http_codes')
->performNoDeepMerging()
->beforeNormalization()
Expand All @@ -1668,9 +1664,9 @@ private function addHttpClientRetrySection()
->end()
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
->integerNode('delay')->defaultValue(1000)->min(0)->info('Time in ms to delay (or the initial value when multiplier is used)')->end()
->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: (delay * (multiple ^ retries))')->end()
->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: delay * (multiple ^ retries)')->end()
->integerNode('max_delay')->defaultValue(0)->min(0)->info('Max time in ms that a retry should ever be delayed (0 = infinite)')->end()
->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1)) to apply to the delay')->end()
->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1) to apply to the delay')->end()
->end()
;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2011,10 +2011,10 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
}

if ($this->isConfigEnabled($container, $retryOptions)) {
$this->registerHttpClientRetry($retryOptions, 'http_client', $container);
$this->registerRetryableHttpClient($retryOptions, 'http_client', $container);
}

$httpClientId = $retryOptions['enabled'] ?? false ? 'http_client.retry.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
$httpClientId = ($retryOptions['enabled'] ?? false) ? 'http_client.retryable.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
foreach ($config['scoped_clients'] as $name => $scopeConfig) {
if ('http_client' === $name) {
throw new InvalidArgumentException(sprintf('Invalid scope name: "%s" is reserved.', $name));
Expand Down Expand Up @@ -2042,7 +2042,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
}

if ($this->isConfigEnabled($container, $retryOptions)) {
$this->registerHttpClientRetry($retryOptions, $name, $container);
$this->registerRetryableHttpClient($retryOptions, $name, $container);
}

$container->registerAliasForArgument($name, HttpClientInterface::class);
Expand All @@ -2062,42 +2062,31 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
}
}

private function registerHttpClientRetry(array $retryOptions, string $name, ContainerBuilder $container)
private function registerRetryableHttpClient(array $options, string $name, ContainerBuilder $container)
{
if (!class_exists(RetryableHttpClient::class)) {
throw new LogicException('Retry failed request support cannot be enabled as version 5.2+ of the HTTP Client component is required.');
throw new LogicException('Support for retrying failed requests requires symfony/http-client 5.2 or higher, try upgrading.');
}

if (null !== $retryOptions['backoff_service']) {
$backoffReference = new Reference($retryOptions['backoff_service']);
if (null !== $options['retry_strategy']) {
$retryStrategy = new Reference($options['retry_strategy']);
} else {
$retryServiceId = $name.'.retry.exponential_backoff';
$retryDefinition = new ChildDefinition('http_client.retry.abstract_exponential_backoff');
$retryDefinition
->replaceArgument(0, $retryOptions['delay'])
->replaceArgument(1, $retryOptions['multiplier'])
->replaceArgument(2, $retryOptions['max_delay'])
->replaceArgument(3, $retryOptions['jitter']);
$container->setDefinition($retryServiceId, $retryDefinition);

$backoffReference = new Reference($retryServiceId);
}
if (null !== $retryOptions['decider_service']) {
$deciderReference = new Reference($retryOptions['decider_service']);
} else {
$retryServiceId = $name.'.retry.decider';
$retryDefinition = new ChildDefinition('http_client.retry.abstract_httpstatuscode_decider');
$retryDefinition
->replaceArgument(0, $retryOptions['http_codes']);
$container->setDefinition($retryServiceId, $retryDefinition);
$retryStrategy = new ChildDefinition('http_client.abstract_retry_strategy');
$retryStrategy
->replaceArgument(0, $options['http_codes'])
->replaceArgument(1, $options['delay'])
->replaceArgument(2, $options['multiplier'])
->replaceArgument(3, $options['max_delay'])
->replaceArgument(4, $options['jitter']);
$container->setDefinition($name.'.retry_strategy', $retryStrategy);

$deciderReference = new Reference($retryServiceId);
$retryStrategy = new Reference($name.'.retry_strategy');
}

$container
->register($name.'.retry', RetryableHttpClient::class)
->register($name.'.retryable', RetryableHttpClient::class)
->setDecoratedService($name, null, -10) // lower priority than TraceableHttpClient
->setArguments([new Reference($name.'.retry.inner'), $deciderReference, $backoffReference, $retryOptions['max_retries'], new Reference('logger')])
->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')])
->addTag('monolog.logger', ['channel' => 'http_client']);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\HttplugClient;
use Symfony\Component\HttpClient\Psr18Client;
use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
use Symfony\Contracts\HttpClient\HttpClientInterface;

return static function (ContainerConfigurator $container) {
Expand Down Expand Up @@ -51,19 +50,14 @@
service(StreamFactoryInterface::class)->ignoreOnInvalid(),
])

// retry
->set('http_client.retry.abstract_exponential_backoff', ExponentialBackOff::class)
->set('http_client.abstract_retry_strategy', GenericRetryStrategy::class)
->abstract()
->args([
abstract_arg('http codes'),
abstract_arg('delay ms'),
abstract_arg('multiplier'),
abstract_arg('max delay ms'),
abstract_arg('jitter'),
])
->set('http_client.retry.abstract_httpstatuscode_decider', HttpStatusCodeDecider::class)
->abstract()
->args([
abstract_arg('http codes'),
])
;
};
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,7 @@
<xsd:element name="http-code" type="xsd:integer" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="enabled" type="xsd:boolean" />
<xsd:attribute name="backoff-service" type="xsd:string" />
<xsd:attribute name="decider-service" type="xsd:string" />
<xsd:attribute name="retry-strategy" type="xsd:string" />
<xsd:attribute name="max-retries" type="xsd:integer" />
<xsd:attribute name="delay" type="xsd:integer" />
<xsd:attribute name="multiplier" type="xsd:float" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
'http_client' => [
'default_options' => [
'retry_failed' => [
'backoff_service' => null,
'decider_service' => null,
'retry_strategy' => null,
'http_codes' => [429, 500],
'max_retries' => 2,
'delay' => 100,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ framework:
http_client:
default_options:
retry_failed:
backoff_service: null
decider_service: null
retry_strategy: null
http_codes: [429, 500]
max_retries: 2
delay: 100
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1500,15 +1500,15 @@ public function testHttpClientRetry()
}
$container = $this->createContainerFromFile('http_client_retry');

$this->assertSame([429, 500], $container->getDefinition('http_client.retry.decider')->getArgument(0));
$this->assertSame(100, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(0));
$this->assertSame(2, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(1));
$this->assertSame(0, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(2));
$this->assertSame(0.3, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(3));
$this->assertSame(2, $container->getDefinition('http_client.retry')->getArgument(3));

$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retry')->getClass());
$this->assertSame(4, $container->getDefinition('foo.retry.exponential_backoff')->getArgument(1));
$this->assertSame([429, 500], $container->getDefinition('http_client.retry_strategy')->getArgument(0));
$this->assertSame(100, $container->getDefinition('http_client.retry_strategy')->getArgument(1));
$this->assertSame(2, $container->getDefinition('http_client.retry_strategy')->getArgument(2));
$this->assertSame(0, $container->getDefinition('http_client.retry_strategy')->getArgument(3));
$this->assertSame(0.3, $container->getDefinition('http_client.retry_strategy')->getArgument(4));
$this->assertSame(2, $container->getDefinition('http_client.retryable')->getArgument(2));

$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retryable')->getClass());
$this->assertSame(4, $container->getDefinition('foo.retry_strategy')->getArgument(2));
}

public function testHttpClientWithQueryParameterKey()
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/HttpClient/Response/AmpResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\HttpClientTrait;
use Symfony\Component\HttpClient\Internal\AmpBody;
use Symfony\Component\HttpClient\Internal\AmpCanary;
use Symfony\Component\HttpClient\Internal\AmpClientState;
use Symfony\Component\HttpClient\Internal\Canary;
use Symfony\Component\HttpClient\Internal\ClientState;
Expand Down
81 changes: 0 additions & 81 deletions src/Symfony/Component/HttpClient/Retry/ExponentialBackOff.php

This file was deleted.

84 changes: 84 additions & 0 deletions src/Symfony/Component/HttpClient/Retry/GenericRetryStrategy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Retry;

use Symfony\Component\HttpClient\Response\AsyncContext;
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;

/**
* Decides to retry the request when HTTP status codes belong to the given list of codes.
*
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class GenericRetryStrategy implements RetryStrategyInterface
{
public const DEFAULT_RETRY_STATUS_CODES = [423, 425, 429, 500, 502, 503, 504, 507, 510];

private $statusCodes;
private $delayMs;
private $multiplier;
private $maxDelayMs;
private $jitter;

/**
* @param array $statusCodes List of HTTP status codes that trigger a retry
* @param int $delayMs Amount of time to delay (or the initial value when multiplier is used)
* @param float $multiplier Multiplier to apply to the delay each time a retry occurs
* @param int $maxDelayMs Maximum delay to allow (0 means no maximum)
* @param float $jitter Probability of randomness int delay (0 = none, 1 = 100% random)
*/
public function __construct(array $statusCodes = self::DEFAULT_RETRY_STATUS_CODES, int $delayMs = 1000, float $multiplier = 2.0, int $maxDelayMs = 0, float $jitter = 0.1)
{
$this->statusCodes = $statusCodes;

if ($delayMs < 0) {
throw new InvalidArgumentException(sprintf('Delay must be greater than or equal to zero: "%s" given.', $delayMs));
}
$this->delayMs = $delayMs;

if ($multiplier < 1) {
throw new InvalidArgumentException(sprintf('Multiplier must be greater than or equal to one: "%s" given.', $multiplier));
}
$this->multiplier = $multiplier;

if ($maxDelayMs < 0) {
throw new InvalidArgumentException(sprintf('Max delay must be greater than or equal to zero: "%s" given.', $maxDelayMs));
}
$this->maxDelayMs = $maxDelayMs;

if ($jitter < 0 || $jitter > 1) {
throw new InvalidArgumentException(sprintf('Jitter must be between 0 and 1: "%s" given.', $jitter));
}
$this->jitter = $jitter;
}

public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
{
return \in_array($context->getStatusCode(), $this->statusCodes, true);
}

public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int
{
$delay = $this->delayMs * $this->multiplier ** $context->getInfo('retry_count');

if ($this->jitter > 0) {
$randomness = $delay * $this->jitter;
$delay = $delay + random_int(-$randomness, +$randomness);
}

if ($delay > $this->maxDelayMs && 0 !== $this->maxDelayMs) {
return $this->maxDelayMs;
}

return (int) $delay;
}
}
Loading

0 comments on commit f52f090

Please sign in to comment.