HttpClientFactory open many connections

[gsGabriel]

I'm using .net core 2.2 and microsoft/dotnet:2.2-aspnetcore-runtime-alpine docker image and used AddHttpClient to create a typed HttpClient

The problem is several open socket connections, even using the factory
I replicated the problem in an application for testing in the following repository: https://github.com/gsGabriel/httpclient-factory/

the connections listed

Could someone tell me what is wrong with my implementation? I can't currently upgrade to .net core 3.0

Originally posted by @gsGabriel in #28842 (comment)

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[ClementeGao]

watching

[ShreyasJejurkar]

I have opened PR for this, please let me know the outcome!

[gsGabriel]

I have opened PR for this, please let me know the outcome!

@MCCshreyas Thank You

The reason I used the code snippet below

            for (int i = 0; i < 100; i++)
            {
                using (var scope = service.CreateScope())
                {
                    var gitHubService = scope.ServiceProvider.GetService<IGitHubService>();
                    tasks.Add(gitHubService.GetAspNetDocsIssues());
                }
            }

it is to simulate the life-cycle of a request, at each request we take an instance of the injection container, so the use of the service provider as it is an endpoint to simulate the problem.

Maybe it's not the best way to test, but I simulate the error only when many requests are made at the endpoint, if the controller has a typed http client, it generates many open socket connections

so I think your PR didn't solve the problem...

[ShreyasJejurkar]

The thing is like all Tasks are getting executed asynchronously at the same time, so everyone started creating socket at very first time, because it's not available at very first time, because of which you get this many sockets open at same time.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[gsGabriel]

The thing is like all Tasks are getting executed asynchronously at the same time, so everyone started creating socket at very first time, because it's not available at very first time, because of which you get this many sockets open at same time.

Okay, you're right ...

but in an application that has a high number of requests per second, wouldn't that be a problem? I believe that the factory should do the control even in a simultaneous scenario

[ShreyasJejurkar]

The thing is like all Tasks are getting executed asynchronously at the same time, so everyone started creating socket at very first time, because it's not available at very first time, because of which you get this many sockets open at same time.

Okay, you're right ...

but in an application that has a high number of requests per second, wouldn't that be a problem? I believe that the factory should do the control even in a simultaneous scenario

How come factory even know that there is an instance if everything started at once asynchronously! And also the code that you have written its not a good practice as well, you should directly use your injected service because you have already add it to DI in startup rather than service provider which is OK, but not needed in this example, as the required service is already present in DI.

[CarnaViire]

@gsGabriel thanks for reporting this! I confirm that I am able to reproduce the issue.

However, it seems that the problem is not on the HttpClientFactory's side. You may observe the same behavior even if you use a single instance of a plain HttpClient.

using System;
using System.Collections.Generic;
using System.Net.Http;
using System.Threading.Tasks;

namespace httpclienthandler_many_connections
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var client = new HttpClient();
            client.BaseAddress = new Uri("https://api.github.com/");
            client.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json");
            client.DefaultRequestHeaders.Add("User-Agent", "httpclienthandler_many_connections");

            var tasks = new List<Task>();
            for (int i = 0; i < 10; i++)
            {
                tasks.Add(GetZenAsync(client));
            }
            await Task.WhenAll(tasks.ToArray());

            Console.ReadKey();
        }

        private static async Task<string> GetZenAsync(HttpClient httpClient)
        {
            var response = await httpClient.GetAsync("/zen");
            var result = await response.Content.ReadAsStringAsync();
            Console.WriteLine(result);
            return result;
        }
    }
}

I wasn't able to find an open issue for that. We will investigate this further.
cc @karelz @dotnet/ncl

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[gsGabriel]

@CarnaViire but this is a known issue of HttpClient, the HttpClientFactory documentation mentioned this.

Shouldn't HttpClientFactory prevent this?

[CarnaViire]

@gsGabriel this is a known issue when using multiple HttpClients, because its default constructor will create a new HttpMessageHandler instance. What HttpClientFactory does is, it ensures that HttpMessageHandlers are pooled and reused between HttpClients. But a single HttpClient would have a single HttpMessageHandler inside, which in theory should also prevent socket exhaustion.

If you are interested, you may see the guidelines for using HttpClient instead of HttpClientFactory here https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-3.1#alternatives-to-ihttpclientfactory

[gsGabriel]

Ah, ok, I understand, thank you very much @CarnaViire :D

[CarnaViire]

So what I discovered is:

1. Having one HttpMessageHandler does not guarantee by default that only one connection per server will be used. In order to guarantee it, you have to set MaxConnectionsPerServer property either in HttpClientHandler or in SocketsHttpHandler

2. Docs for HttpClientFactory also didn't seem to mention using one connection. What I see there in the docs it that it "avoids resource exhaustion problems by pooling HttpMessageHandler instances", so the behavior is compliant with the docs. If there are places that I missed that said about using one connection, please point me to them.

@gsGabriel Did you just wonder why there are several connections open or are you truly experiencing problems with socket exhaustion?

[gsGabriel]

@CarnaViire

Yes, I had the error "too many open files in system" in my linux containers when my application had a high requests per second. In my investigation the reason was the high number of open sockets connections from my http clients, we changed the ulimit of the containers to the maximum value ... solved the problem, but it generated discomfort with the infrastructure team since this does not happen in applications with the same number of requests per second from other technologies.

In my interpretation of the text in relation to the HttpClient life cycle described in httpclient-lifetimes it was understood that it would be reused whenever HttpMessageHandler was available, so I believed that it would not have so many open connections because it would always be reusing.

I will follow your suggestion and use the MaxConnectionsPerServer...

[CarnaViire]

Thanks @gsGabriel and sorry the docs caused misinterpretation!
I am closing the issue then.

[davidfowl]

You shouldn't run out of connections using the IHttpClientFactory normally. I'd like to better understand why you ended up in this situation.

[CarnaViire]

@gsGabriel could you please check and share the following for your application:

1. Whether setting MaxConnectionsPerServer helped with the problem;
2. If you don't set MaxConnectionsPerServer but use a single static instance of HttpClient for the whole application, whether the problem persists.

[gsGabriel]

@CarnaViire

I made several attempts with different implementations. Follow my attempts

1. I tried to set MaxConnectionsPerServer, doesn`t work

            services.AddHttpClient<IGitHubService, GitHubService>(client =>
            {
                client.BaseAddress = new Uri("https://api.github.com/");
            })
            .ConfigureHttpMessageHandlerBuilder((c) =>
                new HttpClientHandler
                {
                    MaxConnectionsPerServer = 1
                }
            );

and

            services.AddHttpClient<IGitHubService, GitHubService>(client =>
            {
                client.BaseAddress = new Uri("https://api.github.com/");
            })
            .ConfigureHttpMessageHandlerBuilder((c) =>
                new SocketsHttpHandler 
                {
                    MaxConnectionsPerServer = 1
                }
            );

1.1 work without set MaxConnectionPerServer if i await operation by operation

2. I tried to use a static class of HttpClient, doesn`t work.
   2.1 work if i await operation by operation

3. I tried to use singleton, doesn`t work
   3.1 work if i await operation by operation

The mention for it to work is if not many connections have been opened

[CarnaViire]

@gsGabriel if you look closely you may notice that when you do

            .ConfigureHttpMessageHandlerBuilder((c) =>
                new SocketsHttpHandler 
                {
                    MaxConnectionsPerServer = 1
                }
            );

you are not actually changing anything -- the ConfigureHttpMessageHandlerBuilder method accepts an action on a builder. To change the handler, you need to use ConfigurePrimaryHttpMessageHandler

services.AddHttpClient<...>(...)
    .ConfigurePrimaryHttpMessageHandler(sp => new SocketsHttpHandler(){ MaxConnectionsPerServer = 1 });

Also, did I understand it right, that you confirm that if you use single static HttpClient, you still get socket exhaustion?

[gsGabriel]

@CarnaViire

Now, following your suggestion, set MaxConnectionsPerServer work`s...
About static HttpClient, yes, i still get socket exhaustion.

[CarnaViire]

Glad to hear MaxConnectionsPerServer works

[gsGabriel]

@CarnaViire Wouldn't it be interesting to add a default to avoid failures?

[CarnaViire]

@gsGabriel What do you think the default should be, so that it satisfies ALL customers? Is 100 a good value, 1K, 10K, 100K, INF? and if yes, why?

[gsGabriel]

@CarnaViire I plan on preventing more connections than the server allows. Linux has a default limit value for simultaneous connections, it opens a file for each connection and if this value is exceeded, it starts to generate errors in the following connections. The default value would be the limit imposed by the server, which is what I believe would satisfies all customers.

[CarnaViire]

By server you mean the server you make a request to, or the machine you are running your application on?

If it's the server you make a request to -- that's no default then, it should be configured every time.

If it's the machine you run on -- as MaxConnectionsPerServer name suggests, it will limit the connections only to a specific address. As soon as you send a request to another address, the total limit becomes twice as large, meaning it's no better than INF. And other applications may run on the same machine, that may require connections.

So it seems that this default limit will satisfy only the case when there's only one application running on the machine and it sends requests to only one address. Unfortunately, I don't see how any other cases will benefit from that.