SteeltoeOSS
Suggest to use HttpClientFactory and about the design of HystrixCommand
- HttpClientFactory is a great design in .NET Core 2.1, I see it is used in Discovery Sample but no in CircuitBreaker Sample (Fortune-Teller-UI). Therefore, suggest to use it in CircuitBreaker Sample too because it can help us create and manage HttpClient well so we no need to care the lifecycle of HttpClient.
- In CircuitBreaker Doc and Sample, I never see a HystrixCommand sample code on calling another API with parameter(s) like the dest API Uri like http://xxxx/api/values/1000, the 1000 is a parameter. Is there any plan to override one method for ExecuteAsync with one parameter (eg.object userState) in HystrixCommand or HystrixCommand<T>? And it is better to pass it to RunAsync and RunFallbackAsync method. If no this design, we only can put the parameter in class level like below, and it might be possible that some multi-thread problems happens in some special scenario.
public class ClientServiceCommand : HystrixCommand<string>
{
IClientService _clientService;
ILogger<ClientServiceCommand> _logger;
private int _clientId;
public ClientServiceCommand(IHystrixCommandOptions options, IClientService clientService,
ILogger<ClientServiceCommand> logger) : base(options)
{
_clientService = clientService;
_logger = logger;
IsFallbackUserDefined = true;
}
public async Task<string> GetClientName(int clientId)
{
_clientId = clientId;
return await ExecuteAsync();
}
protected override async Task<string> RunAsync()
{
var result = await _clientService.GetClientName(_clientId);
_logger.LogInformation("Run: {0}", result);
return result;
}
protected override async Task<string> RunFallbackAsync()
{
_logger.LogInformation("RunFallback");
return await Task.FromResult("Sorry, the service is unavaliable now. Please try again later.");
}
}
Can you please help to advise?
Hi @EdisonChou,
- You're right,
HttpClientFactoryis great and should generally be used - however, we didn't update all of our samples to use it in order to leave a working example of not using it - We have some circuit breakers in the Music Store that might help you out https://github.com/SteeltoeOSS/Samples/tree/dev/MusicStore/src/MusicStoreUI/Services/HystrixCommands
Can you define any specific examples that would demonstrate the multi-threading problem?
Hi @TimHess
Sorry for late reply, thanks for your clarification on not use HttpClientFactory in circuit breaker sample. For the circuit breakers in Music Store, I think it is same as my sample I pasted (use global parameter in class level to pass). For the specific example which might be multi-threading problem, I'd like to share you one scenario (It might not be used commonly in most scenarios). (1) The hystrix command class (Here I use a global parameter in class level -> _clientId):
public class ClientServiceCommand : HystrixCommand<string>
{
IClientService _clientService;
ILogger<ClientServiceCommand> _logger;
private int _clientId;
public ClientServiceCommand(IHystrixCommandOptions options, IClientService clientService,
ILogger<ClientServiceCommand> logger) : base(options)
{
_clientService = clientService;
_logger = logger;
IsFallbackUserDefined = true;
}
public async Task<string> GetClientName(int clientId)
{
_clientId = clientId;
System.Threading.Thread.Sleep(10); // Just used for simulation
_logger.LogDebug("*****Parameter => ClientId: {0}*****", _clientId); // Just used for display message
return await ExecuteAsync();
}
protected override async Task<string> RunAsync()
{
var result = await _clientService.GetClientName(_clientId);
_logger.LogInformation("Run: {0}", result);
return result;
}
protected override async Task<string> RunFallbackAsync()
{
_logger.LogInformation("RunFallback");
return await Task.FromResult("Sorry, the service is unavaliable now. Please try again later.");
}
}
(2) The controller class (Here I simulate 2 task and use Task.WhenAll() method to call the hystrix command) :
[Route("api/[controller]")]
[ApiController]
public class ValuesController : ControllerBase
{
private ClientServiceCommand _clientServiceCommand;
private ILogger<ValuesController> _logger;
public ValuesController(ClientServiceCommand clientServiceCommand, ILogger<ValuesController> logger)
{
_clientServiceCommand = clientServiceCommand;
_logger = logger;
}
// GET api/values
[HttpGet]
public ActionResult<IEnumerable<string>> Get()
{
return new string[] { $"Request Time: {DateTime.Now.ToString()}", "PremiumService-value1", "PremiumService-value2" };
}
// GET api/values/5
[HttpGet("{id}")]
public async Task<string> Get(int id)
{
_logger?.LogInformation($"api/values/{id}");
var task1 = Task.Factory.StartNew(async () =>
{
var result = await _clientServiceCommand.GetClientName(1000);
Console.WriteLine(result);
return result;
});
var task2 = Task.Factory.StartNew(async () =>
{
var result = await _clientServiceCommand.GetClientName(1001);
Console.WriteLine(result);
return result;
});
await Task.WhenAll(task1, task2).ContinueWith(p=>
{
Console.WriteLine("\n-----------------------------------------");
}, TaskContinuationOptions.OnlyOnRanToCompletion);
return "OK";
}
}
After the controller receive one request, will use Task.WhenAll() to trigger 2 tasks to call ClientServiceCommand.GetClientName() asynchronously. The possible problem is that maybe the first task (eg. GetClientName(1000)) has not executed RunAsync() method and the second task (eg. GetClientName(1001)) has changed the parameter to 1001, so both of them will get the same result from RunAsync(1001) instead of RunAysnc(1000) and RunAsync(1001).
Below is my simulation test result for this scenario (Log message shows the 2 tasks will execute the same RunAsync(1001)): Parameter => ClientId: 1001 Parameter => ClientId: 1001
It looks like this issue has grown quite stale (sorry we didn't get back to you in a reasonable time) and Hystrix has entered maintenance mode. If there is any further discussion desired, it should probably happen in https://github.com/SteeltoeOSS/Steeltoe rather than the Samples repo