Increase KestrelServerLimits.KeepAliveTimeout to 130 seconds

[halter73]

HttpClient's SocketsHttpHandler also has a 120 second PooledConnectionIdleTimeout currently. Because Kestrel's KeepAliveTimeout does not exceed this, HttpClient can make a request right as Kestrel closes the connection due to the KeepAliveTimeout. This is one half of attempting to address that issue.

See dotnet/runtime#52267 (comment)

Similar issue for SocketsHttpHandler: dotnet/runtime#52687

[Tratcher]

This is a weird mitigation given HttpClient will have exactly the same issue with other servers like IIS, prior versions of kestrel, etc. If anything, the client should lower their limit.

[geoffkizer]

This is a weird mitigation given HttpClient will have exactly the same issue with other servers like IIS, prior versions of kestrel, etc. If anything, the client should lower their limit.

I'd be happy to lower the client limit. It's currently 120 seconds. 60 seconds seems like it would be more than enough. Thoughts?

[JamesNK]

I'm curious what other servers and clients default to. I'm guessing Kestrel's 120 seconds timeout is from IIS or HttpSys. What does WinHttp have as its timeout?

[geoffkizer]

For HttpWebRequest/ServicePoint, it's 100s. Not sure why we changed this to 120s for SocketsHttpHandler; possibly it just seemed like a rounder number.

I can't seem to find the default for WinHttp, will look further...

[geoffkizer]

Per WinHttp folks, their idle connection timeout is 60s.

[halter73]

I did open a PR for HttpClient at the same time: dotnet/runtime#52687 It's currently changing the default SocketsHttpHandler.DefaultPooledConnectionIdleTimeout from 2 minutes to 1 minute.

[JamesNK]

I think SocketsHttpHandler's timeout should decrease and Kestrel's remain at 120 seconds.

That keeps Kestrel consistent with IIS timeout of 120 seconds - https://docs.microsoft.com/en-us/iis/configuration/system.applicationhost/sites/site/limits#attributes

[halter73]

Ideally, we'll change the IIS default as well. While I'm working to reduce the SocketsHttpHandler's timeout, there's going to be plenty of old HttpClients using SocketsHttpHandler in the wild even if we backport this. Not as many as for WinHttp, but still.

Ultimately, I just don't see the harm in increasing this default by 8%.

[Tratcher]

Ideally, we'll change the IIS default as well.

I don't see that ever happening. This isn't something configured by Asp.Net, it's an IIS and Http.Sys setting.

Stick with the client change.

[halter73]

We can do both. I don't get why we're concerned about increasing the default by 10 seconds.

[JamesNK]

I'm not hard against changing Kestrel.

My POV is I see some small value in having Kestrel and IIS being consistent. If changing SocketsHttpHandler fixes this by itself then why give that up?

[halter73]

Internal discussion:

@Tratcher

Lowering the timeout on the client is a concrete fix, it helps with IIS, Kestrel, and NGinx, and backporting that is a good idea.
Raising the timeout on the server may mitigate the issue for un-updated clients. That's a net benefit, but small enough that I doubt it'd be approved for backporting.

@halter73

I think we agree. It's a change worth making for people who can easily upgrade their servers but not their clients, but it's not something we have to backport on the server imo

So I say lets approve this and move on.

[ghost]

Hello @halter73!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[halter73]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).