Support graceful shutdown in Netty server #3294
Conversation
kciesielski
changed the title
| @@ -2044,7 +2044,8 @@ lazy val examples: ProjectMatrix = (projectMatrix in file("examples")) | |||
| scalaTest.value | |||
| ), | |||
| libraryDependencies ++= loggerDependencies, | |||
| publishArtifact := false | |||
| publishArtifact := false, | |||
| Compile / run / fork := true | |||
There was a problem hiding this comment.
is that temporary for development or a new requirement?
There was a problem hiding this comment.
It is useful for development, when one wants to run examples with examples/runMain a.b.c.HelloExample and terminate a running server.
doc/server/netty.md
Outdated
| @@ -63,6 +63,21 @@ NettyFutureServer(NettyFutureServerOptions.customiseInterceptors.serverLog(None) | |||
| NettyFutureServer(NettyConfig.defaultNoStreaming.socketBacklog(256)) | |||
| ``` | |||
|
|
|||
| ## Graceful shutdown | |||
|
|
|||
| A Netty server has to be properly closed using function `NettyFutureServerBinding.stop()` (and analogous functions available in Cats and ZIO bindings). This function ensures that the server will wait at most 10 seconds for in-flight requests to complete, while rejecting all new requests with 503 during this period. Afterwards, it closes all server resources. | |||
There was a problem hiding this comment.
has -> should? you can still just kill the server ...
| /** Exposes additional `stop` effect, which allows stopping the server inside your test. It will be called after the test anyway (assuming | ||
| * idempotency), but may be useful for some cases where tests need to check specific behavior like returning 503s during shutdown. | ||
| */ | ||
| def serverWithStop(routes: NonEmptyList[ROUTE], gracefulShutdownTimeout: Option[FiniteDuration] = None): Resource[IO, (Port, IO[Unit])] = |
There was a problem hiding this comment.
Maybe that's for a subsequent PR, but maybe it would make sense ot leave this as the only method that has to be implemented by specific test server interpreters? Then server can be implemented in terms of this function, by dropping the second element of the typle
|
|
||
| class ServerGracefulShutdownTests[F[_], OPTIONS, ROUTE](createServerTest: CreateServerTest[F, Any, OPTIONS, ROUTE])(implicit | ||
| m: MonadError[F], | ||
| sleeper: Sleeper[F] |
There was a problem hiding this comment.
maybe this can be a normal parameter?
| def testServer(name: String, rs: => NonEmptyList[ROUTE])( | ||
| runTest: (SttpBackend[IO, Fs2Streams[IO] with WebSockets], Uri) => IO[Assertion] | ||
| ): Test | ||
|
|
||
| def testServerWithStop(name: String, rs: => NonEmptyList[ROUTE], gracefulShutdownTimeout: Option[FiniteDuration])( |
There was a problem hiding this comment.
similarly here ... maybe we can reduce the number of variants here by just supporting the stop-variants, and the others would delegate here
There was a problem hiding this comment.
ah ok they are ... still seems we might have to many similar variants, making this hard to understand
There was a problem hiding this comment.
Yes, I didn't want to mess up other servers, so I thought to make the default implementation implement testServerWithStop in terms of testServer. I'll add some scaladoc to make it less confusing for now.
| @@ -63,6 +63,21 @@ NettyFutureServer(NettyFutureServerOptions.customiseInterceptors.serverLog(None) | |||
| NettyFutureServer(NettyConfig.defaultNoStreaming.socketBacklog(256)) | |||
| ``` | |||
|
|
|||
| ## Graceful shutdown | |||
|
|
|||
| A Netty should can be gracefully closed using function `NettyFutureServerBinding.stop()` (and analogous functions available in Cats and ZIO bindings). This function ensures that the server will wait at most 10 seconds for in-flight requests to complete, while rejecting all new requests with 503 during this period. Afterwards, it closes all server resources. | |||
There was a problem hiding this comment.
should can? ;) maybe just run it through chatgpt to iron out the english :)
Resolves #3000
This PR enhances Netty server with graceful shutdown.
stop()from the server bindingNettyConfigThis feature should work with
Future,ZIO, andCats EffectNetty servers.Tests rely heavily on time, so I'm concerned they may be flaky and need some reworks.