moved setting the base url and the management key header to a common place for all consumers#20
moved setting the base url and the management key header to a common place for all consumers#20smstuebe wants to merge 1 commit intomicrosoft:masterfrom
Conversation
…place for all consumers
|
|
| { | ||
| // Here we set up the stuff that is the same in each http request for our api. | ||
| // In this case it's the base url and the api management key header. | ||
| var client = new HttpClient |
There was a problem hiding this comment.
Can we possibly reuse the HTTP Client if we've already created one? Currently we'd be creating a new client for each request.
There was a problem hiding this comment.
I think you can. I thought about it, but didn't want to introduce side effects regarding DNS cache stuff. If you wan't just add Lazy<HttpClient> :)
There was a problem hiding this comment.
I believe we we're originally new-ing up an instance of HttpClient for every request so your change isn't breaking the existing solution but rather brought to my attention to something we could improve.
I'm guessing the DNS caching side effect you mention is related to the HttpClient instance holding DNS info for 2 minutes? If thats the case then we should* be safe given we use API Management in production and the DNS is locked down and not changing but with that said, I think we can make this all work nicely...
One vs multiple clients
To quote the docs on HttpClient:
HttpClient is intended to be instantiated once and re-used throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors.
Sockets can be held for up to 4 minutes per request so this might actually be causing our network stability issues when we send too many requests. I'd love @robinmanuelthiel to share his thoughts on this as well.
Possible solution to DNS Cache with 1 instance
We could use set the DnsRefreshTimeout property of ServicePointManager class to set our own DNS refresh timeout to something more sensible (if we REALLY needed to). This could help mitigate any issues with DNS caching.
There was a problem hiding this comment.
Don't get me wrong. I'm pro caching, too. I just did not want to change the current behavior :P
3 participants
Pull actual API parameters and technical API parameters apart.