Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add graceful shutdown #10701

Open
wants to merge 15 commits into
base: 4.9.x
Choose a base branch
from
Open

Add graceful shutdown #10701

wants to merge 15 commits into from

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Apr 11, 2024

This PR adds a new interface, GracefulShutdownLifecycle, that parts of the framework can implement to provide graceful shutdown functionality. It is designed to complement LifeCycle.stop(), though the implementations are unrelated.

The netty HTTP server, on graceful shutdown, will stop accepting new connections, and wind down existing ones. HTTP/1 connections are closed as soon as in-progress requests have been served, while HTTP/2 and /3 connections send a GOAWAY packet to indicate to the client that no further requests will be processed.

There is also a readiness health indicator that turns DOWN when a graceful shutdown begins. To keep the health endpoint accessible, there is a new micronaut.server.netty.listeners.*.support-graceful-shutdown config option to disable graceful shutdown for a management port.

Graceful shutdown is triggered by application code through a new GracefulShutdownManager API.

Graceful shutdown is best-effort. In-progress requests may take too long, or HTTP/2 clients may refuse to shut down their connection. For this reason, GracefulShutdownLifecycle returns a CompletionStage. The stage will complete when a graceful shutdown has been achieved, but this may be never, so a user should always add a timeout. After a graceful shutdown (or timeout), a normal LifeCycle.stop should be performed.

Docs are still pending, I want Graeme to review the API before I write it up.

This PR adds a new interface, GracefulShutdownLifecycle, that parts of the framework can implement to provide graceful shutdown functionality. It is designed to complement LifeCycle.stop(), though the implementations are unrelated.

The netty HTTP server, on graceful shutdown, will stop accepting new connections, and wind down existing ones. HTTP/1 connections are closed as soon as in-progress requests have been served, while HTTP/2 and /3 connections send a GOAWAY packet to indicate to the client that no further requests will be processed.

There is also a readiness health indicator that turns DOWN when a graceful shutdown begins. To keep the health endpoint accessible, there is a new `micronaut.server.netty.listeners.*.support-graceful-shutdown` config option to disable graceful shutdown for a management port.

Graceful shutdown is triggered by application code through a new GracefulShutdownManager API.

Graceful shutdown is best-effort. In-progress requests may take too long, or HTTP/2 clients may refuse to shut down their connection. For this reason, GracefulShutdownLifecycle returns a CompletionStage. The stage will complete when a graceful shutdown has been achieved, but this may be never, so a user should always add a timeout. After a graceful shutdown (or timeout), a normal LifeCycle.stop should be performed.

Docs are still pending, I want Graeme to review the API before I write it up.
@yawkat yawkat added the type: enhancement New feature or request label Apr 11, 2024
@yawkat yawkat added this to the 4.5.0 milestone Apr 11, 2024
@yawkat yawkat requested review from graemerocher and sdelamo April 11, 2024 11:05
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Bugs (required ≤ 0)
1 New Critical Issues (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@graemerocher
Copy link
Contributor

Perhaps we should try and address #6493 in this PR as well

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to include documentation about this new API in the main documentation.

@andrebrov
Copy link

@yawkat Hello!

Wanted to see if there are any blockers around this issue since it's a quite important feature.

@yawkat
Copy link
Member Author

yawkat commented Sep 12, 2024

No blockers, it's just on backburner

@altro3
Copy link
Contributor

altro3 commented Oct 11, 2024

Any news about this feature?

@graemerocher
Copy link
Contributor

@yawkat can you prioritise this next again after the client work

@sdelamo sdelamo removed this from the 4.5.0 milestone Oct 28, 2024
# Conflicts:
#	context/build.gradle
#	http-server-netty/src/main/java/io/micronaut/http/server/netty/HttpPipelineBuilder.java
#	http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyHttpServer.java
#	http-server-netty/src/main/java/io/micronaut/http/server/netty/handler/PipeliningServerHandler.java
#	management/src/test/groovy/io/micronaut/management/health/aggregator/HealthAggregatorSpec.groovy
@yawkat yawkat changed the base branch from 4.5.x to 4.9.x February 19, 2025 16:39
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Critical Issues (required ≤ 0)
1 New Bugs (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link
Contributor

@graemerocher graemerocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is the logic that prevents new requests arriving and sends 503 back to the client except for management endpoints?

@Singleton
@Requires(bean = GracefulShutdownManager.class)
@Requires(property = GracefulShutdownConfiguration.ENABLED, value = StringUtils.TRUE, defaultValue = StringUtils.FALSE)
public final class GracefulShutdownListener implements ApplicationEventListener<ShutdownEvent> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make non-public and annotate with @Internal

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this public so people could Replace it in the bean context, not sure if that is a useful thing to do

* @author Jonas Konrad
*/
@Internal
final class GracefulShutdownCapableScheduledThreadPoolExecutor extends ScheduledThreadPoolExecutor implements GracefulShutdownCapable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this only be a activated if graceful shutdown is enabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, all the GracefulShutdownCapable beans are enabled unconditionally. In fact, you can use GracefulShutdownManager without the config option. Only the ShutdownEvent listener is gated.

I think the overhead is negligible, and it allows for more flexibility for programmatic use for those that don't want to use the listener

@yawkat
Copy link
Member Author

yawkat commented Feb 24, 2025

@graemerocher The HTTP listeners stop the server socket so that no new connections are accepted, but existing connections are still served. I think this is better than still accepting connections but returning a status

@graemerocher
Copy link
Contributor

ok can you add docs? Thanks.

@yawkat
Copy link
Member Author

yawkat commented Feb 24, 2025

I'll do the docs tomorrow and then mark this as ready for review

@yawkat yawkat marked this pull request as ready for review February 25, 2025 07:26
@yawkat
Copy link
Member Author

yawkat commented Feb 25, 2025

Not sure what's up with the build, sonar OOM?

@@ -687,6 +697,18 @@ private static String displayAddress(NettyHttpServerConfiguration.NettyListenerC
};
}

public static <T> CompletionStage<T> toCompletionStage(Future<T> future) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be public? Should it be moved to utility class? Seems odd it is here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a utility yes, but it's used in two packages so it has to be public. I wasn't sure where to put it, so I just put it here. The class is internal anyway.

@graemerocher graemerocher requested a review from sdelamo February 25, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
Status: No status
5 participants