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

Provide a way to terminate unfinished requests after graceful shutdown #5941

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Oct 16, 2024

Motivation:

Unfinished requests even after graceful shutdown period are forcibly closed with ClosedSessionException. As ClosedSessionException indicates that the connection was unexpectedly disconnected, ClosedSessionException is not suitable for graceful shutdown.

In this PR, I propose to add ShuttingDownException to terminate unfinished requests when a server stops.

Modifications:

  • Introduce GracefulShutdown to customize graceful shutdown behavior.
    • Users can specify a error function to create an exception to unfinished terminate requests.
  • Fixed HttpServerHandler to send error responses using the error function of GracefulShutdown
  • Fixed Server to send error respones first and then close the connnections.
  • Deprecation) ServerConfig.gracefulShutdownQuietPeriod() and ServerConfig.gracefulShutdownTimeout() have been deprecated in favor of ServerConfig.gracefulShutdown().

Result:

You can now use GracefulShutdown to terminate unfinished requests when a server stops.

GracefulShutdown gracefulShutdown =
  GracefulShutdown
    .builder()
    .quietPeriod(Duration.ofSeconds(10))
    .timeout(Duration.ofSeconds(15))
    .shutdownErrorFunction((ctx, req) -> {
        return new ServerStopException();
    })
    .build();

Server
  .builder()
  .gracefulShutdown(gracefulShutdown);

Motivation:

Unfinished requests even after graceful shutdown period are forcivily
closed with `ClosedSessionException`. As `ClosedSessionException`
indicates that the connection was unexpectedly disconnected,
`ClosedSessionException` is not suitable for graceful shutdown.

In this PR, I propose to add `ShuttingDownException` to terminate
unfinished requests when a server stops.

Modifications:

- Introduce `GracefulShutdown` to customize graceful shutdown behavior.
  - Users can specify a error function to create an exception to
    unfinished terminate requests.
- Fixed `HttpServerHandler` to send error responses using the error
  function of `GracefulShutdown`
- Fixed `Server` to send error respones first and then close the
  connnections.
- Deprecation) `ServerConfig.gracefulShutdownQuietPeriod()` and
  `ServerConfig.gracefulShutdownTimeout()` have been deprecated in favor
  of `ServerConfig.gracefulShutdown()`.

Result:

You can now use `GracefulShutdown` to terminate unfinished requests when
a server stops.
```java
GracefulShutdown gracefulShutdown =
  GracefulShutdown
    .builder()
    .quietPeriod(Duration.ofSeconds(10))
    .timeout(Duration.ofSeconds(15))
    .shutdownErrorFunction((ctx, req) -> {
        return new ServerStopException();
    })
    .build();

Server
  .builder()
  .gracefulShutdown(gracefulShutdown);
```
@ikhoon ikhoon added this to the 1.31.0 milestone Oct 16, 2024
@ikhoon ikhoon marked this pull request as ready for review October 23, 2024 02:52
Comment on lines +47 to +58
/**
* Returns the quiet period to wait for active requests to go end before shutting down.
* {@link Duration#ZERO} means the server will stop right away without waiting.
*/
Duration quietPeriod();

/**
* Returns the amount of time to wait before shutting down the server regardless of active requests.
* This should be set to a time greater than {@code quietPeriod} to ensure the server shuts down even
* if there is a stuck request.
*/
Duration timeout();
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this even more flexible rather than just letting a user configure the quiet period and timeout? For example:

interface GracefulShutdown {
    ...

    // Armeria core passes its GracefulShutdownHandler to this method.
    void startGracefulShutdown(GracefulShutdownHandler handler);
}

// A GracefulShutdown implementation calls back Armeria core via this handler.
interface GracefulShutdownHandler {
    void gracefulShutdownStarted(...);
    void quietPeriodComplete(...);
    void gracefulShutdownComplete(...);
    @Nullable
    Throwable toException(ctx, req); 
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants