Skip to content

Change how JPAConfig cleans up its resources #46357

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

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import java.util.stream.Collectors;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Default;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Singleton;
import jakarta.persistence.AttributeConverter;
import jakarta.transaction.TransactionManager;
Expand All @@ -37,11 +35,9 @@
import org.jboss.jandex.DotName;
import org.jboss.jandex.FieldInfo;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.ParameterizedType;
import org.jboss.jandex.Type;
import org.objectweb.asm.ClassVisitor;

import io.agroal.api.AgroalDataSource;
import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
Expand Down Expand Up @@ -166,24 +162,7 @@ void generateJpaConfigBean(HibernateOrmRecorder recorder,
.scope(Singleton.class)
.unremovable()
.setRuntimeInit()
.supplier(recorder.jpaConfigSupplier())
.destroyer(JPAConfig.Destroyer.class);

// Add a synthetic dependency from JPAConfig to any datasource/pool,
// so that JPAConfig is destroyed before the datasource/pool.
// The alternative would be adding an application destruction observer
// (@Observes @BeforeDestroyed(ApplicationScoped.class)) to JPAConfig,
// but that would force initialization of JPAConfig upon application shutdown,
// which may cause cascading failures if the shutdown happened before JPAConfig was initialized.
if (capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
new Type[] { ClassType.create(DotName.createSimple("io.vertx.sqlclient.Pool")) }, null),
AnnotationInstance.builder(Any.class).build());
} else {
configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
new Type[] { ClassType.create(DotName.createSimple(AgroalDataSource.class)) }, null),
AnnotationInstance.builder(Any.class).build());
}
.supplier(recorder.jpaConfigSupplier());

syntheticBeanBuildItemBuildProducer.produce(configurator.done());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
import io.quarkus.deployment.builditem.LogCategoryBuildItem;
import io.quarkus.deployment.builditem.NativeImageFeatureBuildItem;
import io.quarkus.deployment.builditem.ServiceStartBuildItem;
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.deployment.builditem.SystemPropertyBuildItem;
import io.quarkus.deployment.builditem.TransformedClassesBuildItem;
import io.quarkus.deployment.builditem.nativeimage.NativeImageProxyDefinitionBuildItem;
Expand Down Expand Up @@ -678,9 +679,10 @@ public PersistenceProviderSetUpBuildItem setupPersistenceProvider(
@Consume(PersistenceProviderSetUpBuildItem.class)
@Record(RUNTIME_INIT)
public ServiceStartBuildItem startPersistenceUnits(HibernateOrmRecorder recorder, BeanContainerBuildItem beanContainer,
JpaModelBuildItem jpaModel) {
JpaModelBuildItem jpaModel,
ShutdownContextBuildItem shutdownContextBuildItem) {
if (hasEntities(jpaModel)) {
recorder.startAllPersistenceUnits(beanContainer.getValue());
recorder.startAllPersistenceUnits(beanContainer.getValue(), shutdownContextBuildItem);
}

return new ServiceStartBuildItem("Hibernate ORM");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import io.quarkus.hibernate.orm.runtime.schema.SchemaManagementIntegrator;
import io.quarkus.hibernate.orm.runtime.tenant.DataSourceTenantConnectionResolver;
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.ShutdownContext;
import io.quarkus.runtime.annotations.Recorder;

/**
Expand Down Expand Up @@ -106,8 +107,16 @@ public Supplier<JPAConfig> jpaConfigSupplier() {
return () -> new JPAConfig(runtimeConfig.getValue());
}

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

public Function<SyntheticCreationalContext<SessionFactory>, SessionFactory> sessionFactorySupplier(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;

import jakarta.enterprise.context.spi.CreationalContext;
import jakarta.inject.Inject;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.Persistence;

import org.jboss.logging.Logger;

import io.quarkus.arc.BeanDestroyer;
import io.quarkus.hibernate.orm.runtime.boot.QuarkusPersistenceUnitDescriptor;

public class JPAConfig {
Expand Down Expand Up @@ -129,18 +127,22 @@ public boolean getRequestScopedSessionEnabled() {
return this.requestScopedSessionEnabled;
}

public static class Destroyer implements BeanDestroyer<JPAConfig> {
@Override
public void destroy(JPAConfig instance, CreationalContext<JPAConfig> creationalContext, Map<String, Object> params) {
for (LazyPersistenceUnit factory : instance.persistenceUnits.values()) {
void shutdown() {
LOGGER.trace("Starting to shut down Hibernate ORM persistence units.");
for (LazyPersistenceUnit factory : this.persistenceUnits.values()) {
if (factory.isStarted()) {
try {
LOGGER.tracef("Closing Hibernate ORM persistence unit: %s.", factory.name);
factory.close();
} catch (Exception e) {
LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);
}
} else {
LOGGER.tracef("Skipping Hibernate ORM persistence unit, that failed to start: %s.", factory.name);
}
instance.persistenceUnits.clear();
}
this.persistenceUnits.clear();
LOGGER.trace("Finished shutting down Hibernate ORM persistence units.");
}

static final class LazyPersistenceUnit {
Expand Down Expand Up @@ -175,6 +177,10 @@ public synchronized void close() {
emf.close();
}
}

boolean isStarted() {
return !closed && value != null;
}
}

}
36 changes: 36 additions & 0 deletions extensions/hibernate-search-orm-outbox-polling/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
</dependency>

<!-- test dependencies -->
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-deployment</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5-internal</artifactId>
Expand All @@ -51,6 +56,11 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand All @@ -77,6 +87,32 @@
<configuration>
<skip>true</skip>
</configuration>
<executions>
<execution>
<id>default-test</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<!-- These tests seem to leak metaspace memory
so we run them in a separate, short-lived JVM -->
<excludedGroups>devmode</excludedGroups>
</configuration>
</execution>
<execution>
<id>devmode-test</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<!-- These tests seem to leak metaspace memory
so we run them in a separate, short-lived JVM -->
<groups>devmode</groups>
<!-- We could consider setting reuseForks=false if we
really need to run every single test in its own JVM -->
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package io.quarkus.hibernate.search.orm.outboxpolling.test.configuration.devmode;

import static org.assertj.core.api.Assertions.assertThat;

import java.nio.file.Files;
import java.nio.file.Paths;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusDevModeTest;
import io.restassured.RestAssured;

@Tag("devmode")
public class HibernateSearchDevModeFailingOrmTest {

@RegisterExtension
static final QuarkusDevModeTest config = new QuarkusDevModeTest()
.withApplicationRoot((jar) -> jar
.addClasses(HibernateSearchOutboxPollingTestResource.class,
HibernateSearchOutboxPollingTestResource.Person.class,
HibernateSearchOutboxPollingTestResource.OutboxPollingTestUtils.class)
.addAsResource("application-dev-mode.properties", "application.properties"))
.setLogRecordPredicate(r -> true);

static String APPLICATION_PROPERTIES;

@BeforeAll
static void beforeAll() throws Exception {
APPLICATION_PROPERTIES = Files
.readString(
Paths.get(HibernateSearchDevModeFailingOrmTest.class.getResource("/application-dev-mode.properties")
.toURI()));
}

@Test
public void smoke() {
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(200);

// now add a property that will fail the datasource:
config.modifyResourceFile(
"application.properties",
s -> APPLICATION_PROPERTIES + """
quarkus.datasource.jdbc.min-size=20
""");
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(500);

// add any change to get the shutdown of a failed app completed:
config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);

RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(200);

// At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
// Hence we expect the following things logged:
// initial run:
// - profile activated (after a successful startup)
// - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
// first reload:
// - ORM message telling us that the PU closing won't happen as the PU failed to start
// second reload:
// - profile activated (after a successful startup)
// - no ORM shutdown message, as that will happen after the test body finishes.
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
r -> {
assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
assertThat(r.getParameters()).containsExactly("<default>");
});
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
r -> {
assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
assertThat(r.getParameters()).containsExactly("<default>");
});
assertThat(config.getLogRecords().stream()
.filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
.hasSize(2);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package io.quarkus.hibernate.search.orm.outboxpolling.test.configuration.devmode;

import static org.assertj.core.api.Assertions.assertThat;

import java.nio.file.Files;
import java.nio.file.Paths;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusDevModeTest;
import io.restassured.RestAssured;

@Tag("devmode")
public class HibernateSearchDevModeFailingSearchTest {

@RegisterExtension
static final QuarkusDevModeTest config = new QuarkusDevModeTest()
.withApplicationRoot((jar) -> jar
.addClasses(HibernateSearchOutboxPollingTestResource.class,
HibernateSearchOutboxPollingTestResource.Person.class,
HibernateSearchOutboxPollingTestResource.OutboxPollingTestUtils.class)
.addAsResource("application-dev-mode.properties", "application.properties"))
.setLogRecordPredicate(r -> true);

static String APPLICATION_PROPERTIES;

@BeforeAll
static void beforeAll() throws Exception {
APPLICATION_PROPERTIES = Files
.readString(Paths
.get(HibernateSearchDevModeFailingSearchTest.class.getResource("/application-dev-mode.properties")
.toURI()));
}

@Test
public void smoke() {
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(200);

// now add a property that will fail the search, but since search is started through ORM integrator.. we are actually failing ORM startup:
config.modifyResourceFile(
"application.properties",
s -> APPLICATION_PROPERTIES.replace(
"quarkus.hibernate-search-orm.elasticsearch.hosts=${elasticsearch.hosts:localhost:9200}",
"quarkus.hibernate-search-orm.elasticsearch.hosts=not-a-localhost:9211"));
RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(500);

// and any change to get the shutdown of a failed app completed:
config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);

RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
.statusCode(200);

// At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
// Hence we expect the following things logged:
// initial run:
// - profile activated (after a successful startup)
// - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
// first reload:
// - ORM message telling us that the PU closing won't happen as the PU failed to start
// second reload:
// - profile activated (after a successful startup)
// - no ORM shutdown message, as that will happen after the test body finishes.
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
r -> {
assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
assertThat(r.getParameters()).containsExactly("<default>");
});
assertThat(config.getLogRecords()).satisfiesOnlyOnce(
r -> {
assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
assertThat(r.getParameters()).containsExactly("<default>");
});
assertThat(config.getLogRecords().stream()
.filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
.hasSize(2);
}
}
Loading
Loading