ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.32k stars 1.63k forks source link

.NET Core HttpClient Defects #574

Closed zhaolei0914 closed 6 years ago

zhaolei0914 commented 6 years ago

Expected Behavior / New Feature

Looking at the httpclient data, it is recommended to instantiate httpclient only once, and the entire application uses only one httpclient instance. Is this what Oclot does? My own gateway is using ocelot, and now it's wrong to reward a large amount of concurrency, Error while copying content to a stream,suspected that the httpclient connection pool has run out of mandatory interruptions

Actual Behavior / Motivation for New Feature

i want to know httpclient is right?

Steps to Reproduce the Problem

1. 1. 1.

Specifications

TomPallister commented 6 years ago

@511568420 Ocelot instantiates a HttpClient for each route and then this is shared between all requests to this route. Do not worry HttpClient use is OK :)

We cannot share just one HttpClient because each route can have different delegating handlers, cookie container etc but it is limited to your count of routes. Normally this is a very small number.

zhaolei0914 commented 6 years ago

@TomPallister But it turns out that httpclient reuse uses request. RequestUri. OriginalString as the key to store in the cache, but each request may have a timestamp that causes a different request. RequestUri. OriginalString. Does this result in a lot of httpclient production?

TomPallister commented 6 years ago

@511568420 yes this is a good point the cache will grow. I will take a look and see what we can do!

Deilan commented 6 years ago

Might be worth looking at HttpClientFactory.

TomPallister commented 6 years ago

@Deilan, unfortunately, using this will break a lot of Ocelot functionality as everything is wired up on Startup with HttpClientFactory (last time I checked). At the moment Ocelot can dynamically build HttpClients during runtime. This is important because Ocelot has an administration API which lets you change config on the fly / if you change the config file the internal configuration is rebuilt using the options monitor stuff.

TomPallister commented 6 years ago

Think the fix is, if the user isnt using a ReRoute with a query string then dont use the query string in the cache key. If the user is using a query string in their ReRoute then use it...

zhaolei0914 commented 6 years ago

@TomPallister It is recommended that cached keys do not use query strings, so that the same URl shares a key cache.

TomPallister commented 6 years ago

It’s is the ReRoute that shares the HttpClient not the url, some reroutes can have query string params.

TomPallister commented 6 years ago

Fixed in version 11.0.0