-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Better management of Ebean Shutdown on SIGTERM #475
base: main
Are you sure you want to change the base?
Better management of Ebean Shutdown on SIGTERM #475
Conversation
Can you please
Thanks! |
d61f503
to
530712d
Compare
Done ! 👍 |
Also can you please give the commit a better commit message instead of just |
Current commit message is :
But I will try to add more line to explain the content. 👍 |
No need to add more lines, just the first line should be more meaningful. |
530712d
to
6e2857d
Compare
Commit message changed and comment taken into account. 👍 |
() -> { | ||
databases.forEach((key, database) -> database.shutdown(false, false)); | ||
return CompletableFuture.completedFuture(null); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you actually not just replace this code with the code you added in EbeanLifecycle
? Why do you even need the EbeanLifecycle
class? Isn't it much more straigt forward to change this code here and thats it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mkurz ,
It is clearly possible to change the code in-place.
The objective of adding it to a new class named EbeanLifecycle
is about separation of concern between Ebean lifecycle and Evolution
feature.
Do you want us to change the code in-place and delete EbeanLifecycle
class ?
This pull request aims to fix the problem of Ebean Issue 3420 (ebean-orm/ebean#3420) where SIGTERM signal will immediately shutdown the
ThreadPoolExecutor
of Ebean, causing some reject on async count tasks while HTTP request is in progress.This is solved by calling
ShutdownManager.shutdown()
on application lifecycle stopHook instead of closing each database individually.Note that we have to call
ShutdownManager.deregisterShutdownHook()
to avoid Ebean triggering shutdown on SIGTERM signal.