Skip to content

Commit 03fd33a

Browse files
marko-bekhtamarko-bekhta
committed
Use a shutdown task to clean up ORM resources
1 parent d296cdb commit 03fd33a

File tree

10 files changed

+543
-33
lines changed

10 files changed

+543
-33
lines changed

‎extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
import java.util.stream.Collectors;
1515

1616
import jakarta.enterprise.context.ApplicationScoped;
17-
import jakarta.enterprise.inject.Any;
1817
import jakarta.enterprise.inject.Default;
19-
import jakarta.enterprise.inject.Instance;
2018
import jakarta.inject.Singleton;
2119
import jakarta.persistence.AttributeConverter;
2220
import jakarta.transaction.TransactionManager;
@@ -37,11 +35,9 @@
3735
import org.jboss.jandex.DotName;
3836
import org.jboss.jandex.FieldInfo;
3937
import org.jboss.jandex.MethodInfo;
40-
import org.jboss.jandex.ParameterizedType;
4138
import org.jboss.jandex.Type;
4239
import org.objectweb.asm.ClassVisitor;
4340

44-
import io.agroal.api.AgroalDataSource;
4541
import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
4642
import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
4743
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
@@ -166,24 +162,7 @@ void generateJpaConfigBean(HibernateOrmRecorder recorder,
166162
.scope(Singleton.class)
167163
.unremovable()
168164
.setRuntimeInit()
169-
.supplier(recorder.jpaConfigSupplier())
170-
.destroyer(JPAConfig.Destroyer.class);
171-
172-
// Add a synthetic dependency from JPAConfig to any datasource/pool,
173-
// so that JPAConfig is destroyed before the datasource/pool.
174-
// The alternative would be adding an application destruction observer
175-
// (@Observes @BeforeDestroyed(ApplicationScoped.class)) to JPAConfig,
176-
// but that would force initialization of JPAConfig upon application shutdown,
177-
// which may cause cascading failures if the shutdown happened before JPAConfig was initialized.
178-
if (capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
179-
configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
180-
new Type[] { ClassType.create(DotName.createSimple("io.vertx.sqlclient.Pool")) }, null),
181-
AnnotationInstance.builder(Any.class).build());
182-
} else {
183-
configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
184-
new Type[] { ClassType.create(DotName.createSimple(AgroalDataSource.class)) }, null),
185-
AnnotationInstance.builder(Any.class).build());
186-
}
165+
.supplier(recorder.jpaConfigSupplier());
187166

188167
syntheticBeanBuildItemBuildProducer.produce(configurator.done());
189168
}

‎extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
import io.quarkus.deployment.builditem.LogCategoryBuildItem;
9393
import io.quarkus.deployment.builditem.NativeImageFeatureBuildItem;
9494
import io.quarkus.deployment.builditem.ServiceStartBuildItem;
95+
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
9596
import io.quarkus.deployment.builditem.SystemPropertyBuildItem;
9697
import io.quarkus.deployment.builditem.TransformedClassesBuildItem;
9798
import io.quarkus.deployment.builditem.nativeimage.NativeImageProxyDefinitionBuildItem;
@@ -678,9 +679,10 @@ public PersistenceProviderSetUpBuildItem setupPersistenceProvider(
678679
@Consume(PersistenceProviderSetUpBuildItem.class)
679680
@Record(RUNTIME_INIT)
680681
public ServiceStartBuildItem startPersistenceUnits(HibernateOrmRecorder recorder, BeanContainerBuildItem beanContainer,
681-
JpaModelBuildItem jpaModel) {
682+
JpaModelBuildItem jpaModel,
683+
ShutdownContextBuildItem shutdownContextBuildItem) {
682684
if (hasEntities(jpaModel)) {
683-
recorder.startAllPersistenceUnits(beanContainer.getValue());
685+
recorder.startAllPersistenceUnits(beanContainer.getValue(), shutdownContextBuildItem);
684686
}
685687

686688
return new ServiceStartBuildItem("Hibernate ORM");

‎extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import io.quarkus.hibernate.orm.runtime.schema.SchemaManagementIntegrator;
3737
import io.quarkus.hibernate.orm.runtime.tenant.DataSourceTenantConnectionResolver;
3838
import io.quarkus.runtime.RuntimeValue;
39+
import io.quarkus.runtime.ShutdownContext;
3940
import io.quarkus.runtime.annotations.Recorder;
4041

4142
/**
@@ -106,8 +107,16 @@ public Supplier<JPAConfig> jpaConfigSupplier() {
106107
return () -> new JPAConfig(runtimeConfig.getValue());
107108
}
108109

109-
public void startAllPersistenceUnits(BeanContainer beanContainer) {
110-
beanContainer.beanInstance(JPAConfig.class).startAll();
110+
public void startAllPersistenceUnits(BeanContainer beanContainer, ShutdownContext shutdownContext) {
111+
JPAConfig jpaConfig = beanContainer.beanInstance(JPAConfig.class);
112+
// NOTE:
113+
// - We register the shutdown task before we start any PUs,
114+
// This way we'll be able to clean up even if one of the PUs fails to start while others already did.
115+
// - The step that starts the PUs returns the ServiceStartBuildItem, this in turn ensures that
116+
// the shutdown task that triggers the ShutdownEvent will be registered after this one,
117+
// and users will have access to the "ORM stuff" in their listeners.
118+
shutdownContext.addShutdownTask(jpaConfig::shutdown);
119+
jpaConfig.startAll();
111120
}
112121

113122
public Function<SyntheticCreationalContext<SessionFactory>, SessionFactory> sessionFactorySupplier(

‎extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/JPAConfig.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@
1010
import java.util.concurrent.CompletableFuture;
1111
import java.util.concurrent.ExecutionException;
1212

13-
import jakarta.enterprise.context.spi.CreationalContext;
1413
import jakarta.inject.Inject;
1514
import jakarta.persistence.EntityManagerFactory;
1615
import jakarta.persistence.Persistence;
1716

1817
import org.jboss.logging.Logger;
1918

20-
import io.quarkus.arc.BeanDestroyer;
2119
import io.quarkus.hibernate.orm.runtime.boot.QuarkusPersistenceUnitDescriptor;
2220

2321
public class JPAConfig {
@@ -129,18 +127,22 @@ public boolean getRequestScopedSessionEnabled() {
129127
return this.requestScopedSessionEnabled;
130128
}
131129

132-
public static class Destroyer implements BeanDestroyer<JPAConfig> {
133-
@Override
134-
public void destroy(JPAConfig instance, CreationalContext<JPAConfig> creationalContext, Map<String, Object> params) {
135-
for (LazyPersistenceUnit factory : instance.persistenceUnits.values()) {
130+
void shutdown() {
131+
LOGGER.trace("Starting to shut down Hibernate ORM persistence units.");
132+
for (LazyPersistenceUnit factory : this.persistenceUnits.values()) {
133+
if (factory.isStarted()) {
136134
try {
135+
LOGGER.tracef("Closing Hibernate ORM persistence unit: %s.", factory.name);
137136
factory.close();
138137
} catch (Exception e) {
139138
LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);
140139
}
140+
} else {
141+
LOGGER.tracef("Skipping Hibernate ORM persistence unit, that failed to start: %s.", factory.name);
141142
}
142-
instance.persistenceUnits.clear();
143143
}
144+
this.persistenceUnits.clear();
145+
LOGGER.trace("Finished shutting down Hibernate ORM persistence units.");
144146
}
145147

146148
static final class LazyPersistenceUnit {
@@ -175,6 +177,10 @@ public synchronized void close() {
175177
emf.close();
176178
}
177179
}
180+
181+
boolean isStarted() {
182+
return !closed && value != null;
183+
}
178184
}
179185

180186
}

‎extensions/hibernate-search-orm-outbox-polling/deployment/pom.xml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@
3131
</dependency>
3232

3333
<!-- test dependencies -->
34+
<dependency>
35+
<groupId>io.quarkus</groupId>
36+
<artifactId>quarkus-resteasy-deployment</artifactId>
37+
<scope>test</scope>
38+
</dependency>
3439
<dependency>
3540
<groupId>io.quarkus</groupId>
3641
<artifactId>quarkus-junit5-internal</artifactId>
@@ -51,6 +56,11 @@
5156
<artifactId>assertj-core</artifactId>
5257
<scope>test</scope>
5358
</dependency>
59+
<dependency>
60+
<groupId>io.rest-assured</groupId>
61+
<artifactId>rest-assured</artifactId>
62+
<scope>test</scope>
63+
</dependency>
5464
</dependencies>
5565

5666
<build>
@@ -77,6 +87,32 @@
7787
<configuration>
7888
<skip>true</skip>
7989
</configuration>
90+
<executions>
91+
<execution>
92+
<id>default-test</id>
93+
<goals>
94+
<goal>test</goal>
95+
</goals>
96+
<configuration>
97+
<!-- These tests seem to leak metaspace memory
98+
so we run them in a separate, short-lived JVM -->
99+
<excludedGroups>devmode</excludedGroups>
100+
</configuration>
101+
</execution>
102+
<execution>
103+
<id>devmode-test</id>
104+
<goals>
105+
<goal>test</goal>
106+
</goals>
107+
<configuration>
108+
<!-- These tests seem to leak metaspace memory
109+
so we run them in a separate, short-lived JVM -->
110+
<groups>devmode</groups>
111+
<!-- We could consider setting reuseForks=false if we
112+
really need to run every single test in its own JVM -->
113+
</configuration>
114+
</execution>
115+
</executions>
80116
</plugin>
81117
</plugins>
82118
</build>

‎extensions/hibernate-search-orm-outbox-polling/deployment/src/test/java/io/quarkus/hibernate/search/orm/outboxpolling/test/configuration/devmode/HibernateSearchDevModeFailingOrmTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package io.quarkus.hibernate.search.orm.outboxpolling.test.configuration.devmode;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.nio.file.Files;
6+
import java.nio.file.Paths;
7+
8+
import org.junit.jupiter.api.BeforeAll;
9+
import org.junit.jupiter.api.Tag;
10+
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.api.extension.RegisterExtension;
12+
13+
import io.quarkus.test.QuarkusDevModeTest;
14+
import io.restassured.RestAssured;
15+
16+
@Tag("devmode")
17+
public class HibernateSearchDevModeFailingOrmTest {
18+
19+
@RegisterExtension
20+
static final QuarkusDevModeTest config = new QuarkusDevModeTest()
21+
.withApplicationRoot((jar) -> jar
22+
.addClasses(HibernateSearchOutboxPollingTestResource.class,
23+
HibernateSearchOutboxPollingTestResource.Person.class,
24+
HibernateSearchOutboxPollingTestResource.OutboxPollingTestUtils.class)
25+
.addAsResource("application-dev-mode.properties", "application.properties"))
26+
.setLogRecordPredicate(r -> true);
27+
28+
static String APPLICATION_PROPERTIES;
29+
30+
@BeforeAll
31+
static void beforeAll() throws Exception {
32+
APPLICATION_PROPERTIES = Files
33+
.readString(
34+
Paths.get(HibernateSearchDevModeFailingOrmTest.class.getResource("/application-dev-mode.properties")
35+
.toURI()));
36+
}
37+
38+
@Test
39+
public void smoke() {
40+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
41+
.statusCode(200);
42+
43+
// now add a property that will fail the datasource:
44+
config.modifyResourceFile(
45+
"application.properties",
46+
s -> APPLICATION_PROPERTIES + """
47+
quarkus.datasource.jdbc.min-size=20
48+
""");
49+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
50+
.statusCode(500);
51+
52+
// add any change to get the shutdown of a failed app completed:
53+
config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);
54+
55+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
56+
.statusCode(200);
57+
58+
// At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
59+
// Hence we expect the following things logged:
60+
// initial run:
61+
// - profile activated (after a successful startup)
62+
// - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
63+
// first reload:
64+
// - ORM message telling us that the PU closing won't happen as the PU failed to start
65+
// second reload:
66+
// - profile activated (after a successful startup)
67+
// - no ORM shutdown message, as that will happen after the test body finishes.
68+
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
69+
r -> {
70+
assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
71+
assertThat(r.getParameters()).containsExactly("<default>");
72+
});
73+
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
74+
r -> {
75+
assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
76+
assertThat(r.getParameters()).containsExactly("<default>");
77+
});
78+
assertThat(config.getLogRecords().stream()
79+
.filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
80+
.hasSize(2);
81+
}
82+
}

‎extensions/hibernate-search-orm-outbox-polling/deployment/src/test/java/io/quarkus/hibernate/search/orm/outboxpolling/test/configuration/devmode/HibernateSearchDevModeFailingSearchTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package io.quarkus.hibernate.search.orm.outboxpolling.test.configuration.devmode;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.nio.file.Files;
6+
import java.nio.file.Paths;
7+
8+
import org.junit.jupiter.api.BeforeAll;
9+
import org.junit.jupiter.api.Tag;
10+
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.api.extension.RegisterExtension;
12+
13+
import io.quarkus.test.QuarkusDevModeTest;
14+
import io.restassured.RestAssured;
15+
16+
@Tag("devmode")
17+
public class HibernateSearchDevModeFailingSearchTest {
18+
19+
@RegisterExtension
20+
static final QuarkusDevModeTest config = new QuarkusDevModeTest()
21+
.withApplicationRoot((jar) -> jar
22+
.addClasses(HibernateSearchOutboxPollingTestResource.class,
23+
HibernateSearchOutboxPollingTestResource.Person.class,
24+
HibernateSearchOutboxPollingTestResource.OutboxPollingTestUtils.class)
25+
.addAsResource("application-dev-mode.properties", "application.properties"))
26+
.setLogRecordPredicate(r -> true);
27+
28+
static String APPLICATION_PROPERTIES;
29+
30+
@BeforeAll
31+
static void beforeAll() throws Exception {
32+
APPLICATION_PROPERTIES = Files
33+
.readString(Paths
34+
.get(HibernateSearchDevModeFailingSearchTest.class.getResource("/application-dev-mode.properties")
35+
.toURI()));
36+
}
37+
38+
@Test
39+
public void smoke() {
40+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
41+
.statusCode(200);
42+
43+
// now add a property that will fail the search, but since search is started through ORM integrator.. we are actually failing ORM startup:
44+
config.modifyResourceFile(
45+
"application.properties",
46+
s -> APPLICATION_PROPERTIES.replace(
47+
"quarkus.hibernate-search-orm.elasticsearch.hosts=${elasticsearch.hosts:localhost:9200}",
48+
"quarkus.hibernate-search-orm.elasticsearch.hosts=not-a-localhost:9211"));
49+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
50+
.statusCode(500);
51+
52+
// and any change to get the shutdown of a failed app completed:
53+
config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);
54+
55+
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
56+
.statusCode(200);
57+
58+
// At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
59+
// Hence we expect the following things logged:
60+
// initial run:
61+
// - profile activated (after a successful startup)
62+
// - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
63+
// first reload:
64+
// - ORM message telling us that the PU closing won't happen as the PU failed to start
65+
// second reload:
66+
// - profile activated (after a successful startup)
67+
// - no ORM shutdown message, as that will happen after the test body finishes.
68+
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
69+
r -> {
70+
assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
71+
assertThat(r.getParameters()).containsExactly("<default>");
72+
});
73+
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
74+
r -> {
75+
assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
76+
assertThat(r.getParameters()).containsExactly("<default>");
77+
});
78+
assertThat(config.getLogRecords().stream()
79+
.filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
80+
.hasSize(2);
81+
}
82+
}

0 commit comments

Comments
 (0)