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

Better management of Ebean Shutdown on SIGTERM #475

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions play-ebean/src/main/java/play/db/ebean/EBeanComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@
public interface EBeanComponents extends ConfigurationComponents, DBComponents {

default DynamicEvolutions dynamicEvolutions() {
return new EbeanDynamicEvolutions(
ebeanConfig(), environment(), applicationLifecycle(), evolutionsConfig());
return new EbeanDynamicEvolutions(ebeanConfig(), environment(), evolutionsConfig());
}

default EbeanConfig ebeanConfig() {
return new DefaultEbeanConfig.EbeanConfigParser(config(), environment(), dbApi()).get();
}

default EbeanLifecycle ebeanLifecycle() {
return new EbeanLifecycle(applicationLifecycle());
}

default EvolutionsConfig evolutionsConfig() {
return new DefaultEvolutionsConfigParser(configuration()).parse();
}
Expand Down
19 changes: 12 additions & 7 deletions play-ebean/src/main/java/play/db/ebean/EbeanDynamicEvolutions.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.nio.file.Files;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import javax.inject.Inject;
import javax.inject.Singleton;
import play.Environment;
Expand All @@ -34,21 +33,27 @@ public class EbeanDynamicEvolutions extends DynamicEvolutions {

private final Map<String, Database> databases = new HashMap<>();

@Inject
/**
* @deprecated No need to pass lifecycle anymore as shutdown is now managed by {@link
* play.db.ebean.EbeanLifecycle}. Use {@link #EbeanDynamicEvolutions(EbeanConfig, Environment,
* EvolutionsConfig)} instead.
*/
@Deprecated
public EbeanDynamicEvolutions(
EbeanConfig config,
Environment environment,
ApplicationLifecycle lifecycle,
EvolutionsConfig evolutionsConfig) {
AntoineDuComptoirDesPharmacies marked this conversation as resolved.
Show resolved Hide resolved
this(config, environment, evolutionsConfig);
}

@Inject
public EbeanDynamicEvolutions(
EbeanConfig config, Environment environment, EvolutionsConfig evolutionsConfig) {
this.config = config;
this.environment = environment;
this.evolutionsConfig = evolutionsConfig;
start();
lifecycle.addStopHook(
() -> {
databases.forEach((key, database) -> database.shutdown(false, false));
return CompletableFuture.completedFuture(null);
});
Copy link
Member

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?

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 ?

}

/** Initialise the Ebean servers/databases. */
Expand Down
26 changes: 26 additions & 0 deletions play-ebean/src/main/java/play/db/ebean/EbeanLifecycle.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright (C) from 2022 The Play Framework Contributors <https://github.com/playframework>, 2011-2021 Lightbend Inc. <https://www.lightbend.com>
*/

package play.db.ebean;

import io.ebean.event.ShutdownManager;
import java.util.concurrent.CompletableFuture;
import javax.inject.Inject;
import javax.inject.Singleton;
import play.api.db.evolutions.DynamicEvolutions;
import play.inject.ApplicationLifecycle;

/** A Play module that automatically manages Ebean configuration. */
@Singleton
public class EbeanLifecycle extends DynamicEvolutions {
@Inject
public EbeanLifecycle(ApplicationLifecycle lifecycle) {
ShutdownManager.deregisterShutdownHook();
lifecycle.addStopHook(
() -> {
ShutdownManager.shutdown();
return CompletableFuture.completedFuture(null);
});
}
}
3 changes: 2 additions & 1 deletion play-ebean/src/main/java/play/db/ebean/EbeanModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class EbeanModule extends Module {
public Seq<Binding<?>> bindings(Environment environment, Configuration configuration) {
return seq(
bind(DynamicEvolutions.class).to(EbeanDynamicEvolutions.class).eagerly(),
bind(EbeanConfig.class).toProvider(DefaultEbeanConfig.EbeanConfigParser.class).eagerly());
bind(EbeanConfig.class).toProvider(DefaultEbeanConfig.EbeanConfigParser.class).eagerly(),
bind(EbeanLifecycle.class).toSelf().eagerly());
}
}
Loading