Increase default HostOptions.ShutdownTimeout value to 30 seconds

[ReubenBond]

Fixes #63709

See the above for context.

[ghost]

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

Issue Details

---

Fixes #63709

See the above for context.

Author:	ReubenBond
Assignees:	-
Labels:	area-Extensions-Hosting
Milestone:	-

[eerhardt]

I think this reasoning makes sense.

FYI @Tratcher @davidfowl, in case you have any objections.

[eerhardt]

Note that the WebHost still has a 5 second shutdown time out (which is where the generic Host got its default value from):

https://github.com/dotnet/aspnetcore/blob/2e66c71b8ae285bd5b142ad66fb3219f35fbe9dd/src/Hosting/Hosting/src/Internal/WebHostOptions.cs#L68

And just an FYI aspnet/Hosting#1237 is the PR where the Host shutdown timeout was introduced.

[Tratcher]

This is a big jump and somewhat concerning. This can also affect dev inner-loop scenarios.

Ultimately it is subjective and app specific which is why it's configurable. Kubernetes is just one of many scenarios. I'm not sure of a constructive way to pick an appropriate default.

[eerhardt]

This can also affect dev inner-loop scenarios.

Can you elaborate? When would a dev inner-loop scenario hang on shutdown? (beyond a dev mistake, which I'd assume they would want to correct sooner rather than later)

[davidfowl]

It really only affects inner loop scenarios when the app is hanging. If we're concerned about that we could change the defaults in dev vs non dev scenarios.

[Tratcher]

It really only affects inner loop scenarios when the app is hanging.

Right, it's not uncommon to break your shutdown sequence when developing, we used to do it with kestrel all the time. Long running requests would cause a similar issue.

If we're concerned about that we could change the defaults in dev vs non dev scenarios.

Please no, trying to distinguish environments or having multiple timeouts would only make this more complicated.

How do we decide if 30s is really a better value than 5s? Do we have any data about how long apps take to shut down?

[ReubenBond]

How do we decide if 30s is really a better value than 5s? Do we have any data about how long apps take to shut down?

I laid out the principles behind picking a value in the original post. 5s is the outlier - a value of 30s-2min is more typical for a shutdown timeout. No firm data, but I work with teams internal and external and it's common for them to need to increase this to make things work smoothly on Kubernetes.

If an app is hanging on shutdown then it's better that it annoys developers during dev time rather than causing odd/ungraceful behavior in production. I agree that we should change that in templates for local dev (like we do for binding ports and the environment).

We should optimize towards:

• Correct apps work well in production
• Defects are apparent at dev/test time

[Tratcher]

It's better to err towards the value being too high than too low (i.e, we shouldn't optimize for malfunctioning applications).

This is the main premise I want to push on. This limit exists precisely to catch malfunctioning apps and prevent them from having an outside impact. Yes, 5s is short, but how long is enough for the 50th or 90th percentile?

You picked 30s based on Kubernetes, but shouldn't we be slightly lower than their timeout so we can attempt a non-graceful shutdown before the container is killed? E.g. 20 or 25s?

I'm not going to block this any further, I just want to make sure we're not picking an arbitrary value, and that we don't keep changing it on a whim.

[maryamariyan]

Converting to draft until a consensus is made on the conversation here.
@davidfowl @Tratcher what is the drawback in increasing this timeout?