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

preWait not working since Ktor v2.3.0 by default #115

Open
mivanilov opened this issue Feb 5, 2024 · 3 comments
Open

preWait not working since Ktor v2.3.0 by default #115

mivanilov opened this issue Feb 5, 2024 · 3 comments

Comments

@mivanilov
Copy link

mivanilov commented Feb 5, 2024

Since Ktor v2.3.0 shutdown hook was added to Ktor engines e.g. CIO and Netty, making Ktor server to stop before waiting a preWait duration configured by SuspendApp.
Adding an example project to reproduce this issue issue-demo.zip

Steps to reproduce it:

  • Start app with -Dio.ktor.server.engine.ShutdownHook=true (true by default)
  • curl http://localhost/ping -v to get 200 response
  • kill -s SIGTERM `lsof -t -i:80`
  • curl http://localhost/ping -v again to get an error while it is expected to get 200 response for a period of preWait duration before engine.stop is called.

Setting -Dio.ktor.server.engine.ShutdownHook=false fixes this issue.
To save developers time troubleshooting this issue, probably SuspendApp readme/docs should mention this Ktor engine configuration bit making sure SuspendApp works as expected?
Also might be worth adding an integration test to cover this behaviour.

@nomisRev
Copy link
Member

Hey @mivanilov,

Thank you for the report!! We could potentially also manually put Dio.ktor.server.engine.ShutdownHook to false, and print a warning if the user left it to true. 🤔 Or some other kind of logging.

What woud you expect to happen? It's not clear to me from your comment, or would you prefer it to blow up?

@mivanilov
Copy link
Author

Hey @nomisRev looking at the Ktor engines and SuspendApp code as it is now, it makes sense to me set Dio.ktor.server.engine.ShutdownHook to false as ultimately SuspendApp delays engine.stop call - which is the same function that is registered with the Ktor shutdown hook e.g. for Netty https://github.com/ktorio/ktor/blob/main/ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt#L214
Also I would probably prefer it blow up if Dio.ktor.server.engine.ShutdownHook is set to true as in such case SuspendApp just won't work as there will be no expected delay before engine.stop call.

@nomisRev
Copy link
Member

Hey @mivanilov,

Thank you for letting me know your thoughts, I 100% agree. I am going to implement this in the next month or so, while we work on releasing Arrow 2.0 alongside the K2 release that'll come any week now. KotlinConf 👀

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

No branches or pull requests

2 participants