quarkus-bot[bot]
commented
1 year ago /cc @gsmet (hibernate-search)
Open yrodiere opened 1 year ago
quarkus-bot[bot]
commented
1 year ago /cc @gsmet (hibernate-search)
marko-bekhta
commented
3 weeks ago Hey @yrodiere
I've tried looking into this one, and from what it seems this is related to CDI more than to outbox polling and Hibernate Search. And it also looks like a more general problem. As you've suspected io.quarkus.agroal.runtime.DataSources sometimes gets closed sooner than the io.quarkus.hibernate.orm.runtime.JPAConfig leading to that exception.
We are adding an injection point: https://github.com/quarkusio/quarkus/blob/66429bddeacc3ea8cd8e40a03b9906d283553d92/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java#L146-L160
and based on the comment, we expect that the JPAConfig will get closed before the DataSources. I went through the Arc code a bit and didn't find that this would be "enforced" (but I also didn't want to spend too much time looking so maybe I've missed it 🙈)
To check this assumption, I've created two simple app-scoped beans (that is if my understanding was correct):
@ApplicationScoped
public class MyService1 {
@Inject
MyService2 service2;
public String string() {
return service2.string();
}
@PreDestroy
public void destroy() {
System.err.println("===========" + this.getClass().getName() + " destroy");
}
}@ApplicationScoped
public class MyService2 {
public String string() {
return "string2";
}
@PreDestroy
public void destroy() {
System.err.println("===========" + this.getClass().getName() + " destroy");
}
}and from time to time, MyService1 is closed before MyService2... But if I understand the assumption correctly, we'd expect that MyService2 is always closed before MyService1
I see that here, where the beans are destroyed (in the case of the JPAConfig and DataSources we are talking about the singleton context):
we get a set of beans to destroy, and if we are "unlucky", DataSources are getting ahead of the JPAConfig in the iteration, and things fail: E.g.:
Destroy order io.quarkus.arc.impl.SingletonContext@1c231768:
class io.quarkus.agroal.runtime.DataSources DeclaringBean: null
class io.quarkus.narayana.jta.runtime.NarayanaJtaProducers DeclaringBean: CLASS bean [class=io.quarkus.narayana.jta.runtime.NarayanaJtaProducers, id=ol81PNK5K-jPtG8Wb8uxqBBB9cM]
class io.quarkus.arc.runtime.ConfigStaticInitValues DeclaringBean: null
class io.quarkus.it.hibernate.search.orm.elasticsearch.coordination.outboxpolling.HibernateSearchOutboxPollingTestResource DeclaringBean: null
class io.quarkus.datasource.runtime.DataSourceSupport DeclaringBean: null
class io.quarkus.hibernate.orm.runtime.cdi.QuarkusArcBeanContainer DeclaringBean: null
class io.quarkus.hibernate.orm.runtime.JPAConfig DeclaringBean: null
class io.quarkus.arc.runtime.LoggerProducer DeclaringBean: null
class io.quarkus.narayana.jta.runtime.NarayanaJtaProducers DeclaringBean: CLASS bean [class=io.quarkus.narayana.jta.runtime.NarayanaJtaProducers, id=ol81PNK5K-jPtG8Wb8uxqBBB9cM]Neither JPAConfig nor DataSources have a DeclaringBean, hence DataSources are not destroyed in the first loop (with the // Destroy the producers first comment) and both are destroyed in the second one in the "random" order 😕
yrodiere
commented
3 weeks ago Thanks for looking into this @marko-bekhta .
and based on the comment, we expect that the
JPAConfigwill get closed before theDataSources. I went through the Arc code a bit and didn't find that this would be "enforced" (but I also didn't want to spend too much time looking so maybe I've missed it 🙈)
@mkouba @Ladicek can you confirm that Arc beans are closed in an order that is consistent with the dependency graph -- at least when there is no cycle? If not, can you please advise on the best way to ensure Arc beans get closed in a specific order?
FWIW, if closing order is not guaranteed, other issue may have the same root cause; see for example https://github.com/quarkusio/quarkus/issues/36265#issuecomment-1750632017
Ladicek
commented
3 weeks ago There is no order in which beans are destroyed, and I believe there is no way to affect that.
I think @mkouba mentioned this several times: @PreDestroy listeners should be "local", that is, they should not expect other beans to exist. I believe observing the ShutdownEvent is a better choice in this regard -- when the observers run, it is guaranteed that the contexts still exist, and you can define observer priority (ordering).
marko-bekhta
commented
3 weeks ago Thanks @Ladicek I'll take a look at how we can update things then 🥲
yrodiere
commented
3 weeks ago Thanks.
Thanks @Ladicek I'll take a look at how we can update things then 🥲
@marko-bekhta I'm currently making changes in this area in #41929 ; see in particular https://github.com/quarkusio/quarkus/pull/41929/files#diff-ae22f0b05b345fab3d41e46504afedf53b79b6b3b4a12411bde2c8ae60c2c234L445-L453 and https://github.com/quarkusio/quarkus/pull/41929/files#diff-bc8502db2a346fd80de961c88ff8274ddfa740cdc9b462d33021d25cce7c452eR292.
You may want to wait until I'm done before you work on this? I'll probably be less likely to lead to conflicts. Can't promise when I'll be done though :/
I believe observing the ShutdownEvent is a better choice in this regard -- when the observers run, it is guaranteed that the contexts still exist, and you can define observer priority (ordering).
So I guess we could keep the existing @PreDestroy thing for Agroal, and shut down Hibernate ORM/Reactive with a ShutdownEvent observer? Which would guarantee that Hibernate ORM/Reactive shut down before Datasources.
For #36265 we basically want Agroal to shut down after Vertx... that probably means the Vertx extension needs to use ShutdownEvent? Maybe it already does, IDK.
marko-bekhta
commented
3 weeks ago yeah, sure, I can wait 😃 no worries.
mkouba
commented
1 week ago There is no order in which beans are destroyed, and I believe there is no way to affect that.
I think @mkouba mentioned this several times:
@PreDestroylisteners should be "local", that is, they should not expect other beans to exist. I believe observing theShutdownEventis a better choice in this regard -- when the observers run, it is guaranteed that the contexts still exist, and you can define observer priority (ordering).
Correct. For @ApplicationScoped you can also consider an observer of @BeforeDestroyed(ApplicationScoped.class).
Describe the bug
When using the extension
quarkus-hibernate-search-orm-coordination-outbox-pollingand stopping the application in dev mode, exception gets thrown, caused by Hibernate Search not being able to create a connection to the database (it needs to in order to re-register agents).Expected behavior
No exception.
Actual behavior
How to Reproduce?
No response
Output of
uname -aorverNo response
Output of
java -versionNo response
GraalVM version (if different from Java)
No response
Quarkus version or git rev
No response
Build tool (ie. output of
mvnw --versionorgradlew --version)No response
Additional information
We should probably shut down Agroal after Hibernate ORM to avoid this exception.