search

playframework / play-ebean

Play Ebean module
Apache License 2.0
103 stars 69 forks source link

Better management of Ebean Shutdown on SIGTERM #475

Open AntoineDuComptoirDesPharmacies opened 1 week ago

AntoineDuComptoirDesPharmacies commented 1 week ago

This pull request aims to fix the problem of Ebean Issue 3420 (https://github.com/ebean-orm/ebean/issues/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.

mkurz commented 1 week ago

Can you please

  1. Run sbt formatCode
  2. Squash your changes into one single commit on top of main?

Thanks!

AntoineDuComptoirDesPharmacies commented 1 week ago

Can you please

  1. Run sbt formatCode
  2. Squash your changes into one single commit on top of main?

Thanks!

Done ! 👍

mkurz commented 1 week ago

Also can you please give the commit a better commit message instead of just # Ebean 3420? It's hard to tell what the commit does when just looking at the git log.

AntoineDuComptoirDesPharmacies commented 1 week ago

Also can you please give the commit a better commit message instead of just # Ebean 3420? It's hard to tell what the commit does when just looking at the git log.

Current commit message is : 

# Ebean 3420
Create EbeanLifecycle component to manage Ebean shutdown

But I will try to add more line to explain the content. 👍

mkurz commented 1 week ago

Also can you please give the commit a better commit message instead of just # Ebean 3420? It's hard to tell what the commit does when just looking at the git log.

Current commit message is :

# Ebean 3420
Create EbeanLifecycle component to manage Ebean shutdown

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.

AntoineDuComptoirDesPharmacies commented 1 week ago

Commit message changed and comment taken into account. 👍