From 6f8512f434910d9fbbcc44300133e38b5c2940f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ram=C3=B3n=20Tom=C3=A1s?= Date: Wed, 22 Mar 2017 12:24:55 +0100 Subject: [PATCH 1/4] Increased circuit breaker threshold Fix lifescope issue with IHttpClient Adapt circuit breaker to trigger its counter for no success http response --- .../Utilities/HttpApiClientWrapper.cs | 36 +++++++++---------- src/Web/WebMVC/Startup.cs | 4 +-- .../modules/shared/services/data.service.ts | 2 ++ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs index 5dd26a0fb..1e55305c5 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs @@ -8,6 +8,12 @@ using System.Threading.Tasks; namespace WebMVC.Services.Utilities { + /// + /// HttpClient wrapper that integrates Retry and Circuit + /// breaker policies when calling to Api services. + /// Currently is ONLY implemented for the ASP MVC + /// and Xamarin App + /// public class HttpApiClientWrapper : IHttpClient { private HttpClient _client; @@ -49,11 +55,11 @@ namespace WebMVC.Services.Utilities Policy.Handle() .WaitAndRetryAsync( // number of retries - 3, + 3, // exponential backofff retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)), // on retry - (exception, timeSpan, retryCount, context) => + (exception, timeSpan, retryCount, context) => { _logger.LogTrace($"Retry {retryCount} " + $"of {context.PolicyKey} " + @@ -62,27 +68,21 @@ namespace WebMVC.Services.Utilities } ); - // Notice that these (and other methods below) are Task - // returning asynchronous methods. But, they do not - // have the 'async' modifier, and do not contain - // any 'await statements. In each of these methods, - // the only asynchronous call is the last (or only) - // statement of the method. In those instances, - // a Task returning method that does not use the - // async modifier is preferred. The compiler generates - // synchronous code for this method, but returns the - // task from the underlying asynchronous method. The - // generated code does not contain the state machine - // generated for asynchronous methods. public Task GetStringAsync(string uri) => - HttpInvoker(() => _client.GetStringAsync(uri)); + HttpInvoker(() => + _client.GetStringAsync(uri)); public Task PostAsync(string uri, T item) => // a new StringContent must be created for each retry // as it is disposed after each call - HttpInvoker(() =>_client.PostAsync(uri, - new StringContent(JsonConvert.SerializeObject(item), - System.Text.Encoding.UTF8, "application/json"))); + HttpInvoker(() => + { + var response = _client.PostAsync(uri, new StringContent(JsonConvert.SerializeObject(item), System.Text.Encoding.UTF8, "application/json")); + // raise exception if not success response + // needed for circuit breaker to track fails + response.Result.EnsureSuccessStatusCode(); + return response; + }); public Task DeleteAsync(string uri) => HttpInvoker(() => _client.DeleteAsync(uri)); diff --git a/src/Web/WebMVC/Startup.cs b/src/Web/WebMVC/Startup.cs index 7888ae253..ee2412bee 100644 --- a/src/Web/WebMVC/Startup.cs +++ b/src/Web/WebMVC/Startup.cs @@ -54,11 +54,11 @@ namespace Microsoft.eShopOnContainers.WebMVC if(Configuration.GetValue("ActivateCircuitBreaker") == bool.TrueString) { - services.AddSingleton(); + services.AddTransient(); } else { - services.AddSingleton(); + services.AddTransient(); } } diff --git a/src/Web/WebSPA/Client/modules/shared/services/data.service.ts b/src/Web/WebSPA/Client/modules/shared/services/data.service.ts index 785728943..3b121f4e7 100644 --- a/src/Web/WebSPA/Client/modules/shared/services/data.service.ts +++ b/src/Web/WebSPA/Client/modules/shared/services/data.service.ts @@ -11,6 +11,8 @@ import 'rxjs/add/operator/catch'; import { SecurityService } from './security.service'; import { Guid } from '../../../guid'; +// Implementing a Retry-Circuit breaker policy +// is pending to do for the SPA app @Injectable() export class DataService { constructor(private http: Http, private securityService: SecurityService) { } From 449ee3f7a310349cee74856a38b8f6d34482b0bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ram=C3=B3n=20Tom=C3=A1s?= Date: Wed, 22 Mar 2017 12:40:20 +0100 Subject: [PATCH 2/4] Increase Retries in HttpClientWrapper --- src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs index 1e55305c5..06172eb53 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs @@ -36,7 +36,7 @@ namespace WebMVC.Services.Utilities Policy.Handle() .CircuitBreakerAsync( // number of exceptions before breaking circuit - 3, + 5, // time circuit opened before retry TimeSpan.FromMinutes(1), (exception, duration) => @@ -55,7 +55,7 @@ namespace WebMVC.Services.Utilities Policy.Handle() .WaitAndRetryAsync( // number of retries - 3, + 5, // exponential backofff retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)), // on retry From 2e674ad5321fef5bea8ab2a5bef6f31b39d5e44e Mon Sep 17 00:00:00 2001 From: Eduard Tomas Date: Wed, 22 Mar 2017 14:23:25 +0100 Subject: [PATCH 3/4] Replaced Tuple by C#7 multiple return statement Replaced private set only used in ctor by readonly prop --- .../EventBus/Events/IntegrationEvent.cs | 2 +- .../EventBusRabbitMQ/EventBusRabbitMQ.cs | 78 ++++++++++--------- .../EventBusRabbitMQ/EventBusRabbitMQ.csproj | 1 + 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/src/BuildingBlocks/EventBus/EventBus/Events/IntegrationEvent.cs b/src/BuildingBlocks/EventBus/EventBus/Events/IntegrationEvent.cs index 11185764e..e1802704e 100644 --- a/src/BuildingBlocks/EventBus/EventBus/Events/IntegrationEvent.cs +++ b/src/BuildingBlocks/EventBus/EventBus/Events/IntegrationEvent.cs @@ -11,6 +11,6 @@ namespace Microsoft.eShopOnContainers.BuildingBlocks.EventBus.Events Id = Guid.NewGuid(); } - public Guid Id { get; private set; } + public Guid Id { get; } } } diff --git a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs index e31c43396..b850c4eef 100644 --- a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs +++ b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs @@ -21,7 +21,8 @@ namespace Microsoft.eShopOnContainers.BuildingBlocks.EventBusRabbitMQ private readonly Dictionary> _handlers; private readonly List _eventTypes; - private Tuple _connection; + private IModel _model; + private IConnection _connection; private string _queueName; @@ -86,15 +87,15 @@ namespace Microsoft.eShopOnContainers.BuildingBlocks.EventBusRabbitMQ _handlers.Remove(eventName); var eventType = _eventTypes.Single(e => e.Name == eventName); _eventTypes.Remove(eventType); - _connection.Item1.QueueUnbind(queue: _queueName, + _model.QueueUnbind(queue: _queueName, exchange: _brokerName, routingKey: eventName); if (_handlers.Keys.Count == 0) { _queueName = string.Empty; - _connection.Item1.Dispose(); - _connection.Item2.Dispose(); + _model.Dispose(); + _connection.Dispose(); } } @@ -103,50 +104,53 @@ namespace Microsoft.eShopOnContainers.BuildingBlocks.EventBusRabbitMQ public void Dispose() { - if (_connection != null) - { - _handlers.Clear(); - _connection.Item1.Dispose(); - _connection.Item2.Dispose(); - } + _handlers.Clear(); + _model?.Dispose(); + _connection?.Dispose(); } private IModel GetChannel() { - if (_connection != null) + if (_model != null) { - return _connection.Item1; + return _model; } else { - var factory = new ConnectionFactory() { HostName = _connectionString }; - var connection = factory.CreateConnection(); - var channel = connection.CreateModel(); - - channel.ExchangeDeclare(exchange: _brokerName, - type: "direct"); - if (string.IsNullOrEmpty(_queueName)) - { - _queueName = channel.QueueDeclare().QueueName; - } - - var consumer = new EventingBasicConsumer(channel); - consumer.Received += async (model, ea) => - { - var eventName = ea.RoutingKey; - var message = Encoding.UTF8.GetString(ea.Body); - - await ProcessEvent(eventName, message); - }; - channel.BasicConsume(queue: _queueName, - noAck: true, - consumer: consumer); - _connection = new Tuple(channel, connection); - - return _connection.Item1; + ((_model, _connection) = CreateConnection(); + return _model; } } + + private (IModel model, IConnection connection) CreateConnection() + { + var factory = new ConnectionFactory() { HostName = _connectionString }; + var con = factory.CreateConnection(); + var channel = con.CreateModel(); + + channel.ExchangeDeclare(exchange: _brokerName, + type: "direct"); + if (string.IsNullOrEmpty(_queueName)) + { + _queueName = channel.QueueDeclare().QueueName; + } + + var consumer = new EventingBasicConsumer(channel); + consumer.Received += async (model, ea) => + { + var eventName = ea.RoutingKey; + var message = Encoding.UTF8.GetString(ea.Body); + + await ProcessEvent(eventName, message); + }; + channel.BasicConsume(queue: _queueName, + noAck: true, + consumer: consumer); + + return (channel, con); + } + private async Task ProcessEvent(string eventName, string message) { if (_handlers.ContainsKey(eventName)) diff --git a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.csproj b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.csproj index 6e399a627..cf36a2222 100644 --- a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.csproj +++ b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.csproj @@ -9,6 +9,7 @@ + From 20d2e32719dfada74b8dd54de5710edb027ccc11 Mon Sep 17 00:00:00 2001 From: Eduard Tomas Date: Wed, 22 Mar 2017 14:40:00 +0100 Subject: [PATCH 4/4] Ups... too many parens xD --- .../EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs index b850c4eef..3388875ab 100644 --- a/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs +++ b/src/BuildingBlocks/EventBus/EventBusRabbitMQ/EventBusRabbitMQ.cs @@ -117,7 +117,7 @@ namespace Microsoft.eShopOnContainers.BuildingBlocks.EventBusRabbitMQ } else { - ((_model, _connection) = CreateConnection(); + (_model, _connection) = CreateConnection(); return _model; } }