Quarkus: Move some of the Hibernate config to runtime

[gsmet]

So the idea is to move as many as possible elements to runtime config.

Note that some of the elements should stay as build time config because they are used during the build (for instance, the dialect) but we should try to push as many configuration knobs to runtime as long as it's technically doable.

[machi1990]

Hi @gsmet,

I was curios about this one and started checking its feasibility. I have some few questions to help my understanding.

1. What will be the impact of this change to HibernateOrmTemplate.initMetaData(…) which is called on a STATIC_INIT record block?: https://github.com/quarkusio/quarkus/blob/688ee16b0641ac60239c0c7970ce23b6cbff35ff/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java#L206-L209

2. My understanding (correct me if I am wrong) is to be able to access RUN_TIME configurations, the code has to be within a RUNTIME_INIT method?

3. Also, template.initMetadata(…) calls this methods
   https://github.com/quarkusio/quarkus/blob/688ee16b0641ac60239c0c7970ce23b6cbff35ff/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/PersistenceUnitsHolder.java#L29-L51

And the comment says to call this specific method within a STATIC_INIT method (current the case), what will be the implications (provided that I understood point 2 correctly) of calling this within a RUNTIME_INIT method?

Thanks.

[gsmet]

@machi1990 sorry for the late answer. So the whole thing is to try to check which properties have to be defined before static init and which ones can be defined later.

Moving things to runtime init is a no no as it will slow down our startup in native mode and we don't want that.

So we need to figure out for each properties if they need to be defined early (in this case, they have to be defined at build time) or later (in this case, they could be runtime defined).

It's not something obvious as you would need to check when they are used. If you don't feel it, I can work on it later this month.

Not sure if it's totally clear?

[machi1990]

@gsmet thanks for getting back.

Moving things to runtime init is a no no as it will slow down our startup in native mode and we don't want that.

This is what I thought. Thanks for confirming it.

So we need to figure out for each properties if they need to be defined early (in this case, they have to be defined at build time) or later (in this case, they could be runtime defined).

It's not something obvious as you would need to check when they are used. If you don't feel it, I can work on it later this month.

I should have some time next week. I'll start to dig into it and come to you with questions from time to time to see the direction I have taken.

Not sure if it's totally clear?

It is pretty clear and answers my questions. Thanks.

[Sanne]

we should try to push as many configuration knobs to runtime as long as it's technically doable.

I don't think that should be the goal. There's several things that technically could be done at runtime, and yet we shouldn't.

Let's try to make a list; clearly the need expressed in #5769 is important, but many others are not and would hurt optimisation potential.

The strategy in Quarkus is that all options are build-time, unless there's a good reason to have it overridable at runtime.

[gsmet]

That's exactly what I was saying by "technically doable".

If it allows for some optimizations, then it's not doable as we need it in earlier steps and then should stay build time.

But all the configuration knobs that are clearly not tied to build time AND that we can define later on the Hibernate side (some of them might be needed when we build the metadata) should be runtime.

So I think we are on the same page here.

[Sanne]

Ok glad to hear we're on the same page. Mind you, "technically doable" is much broader so I wouldn't use the expression in this context.

For example Emmanuel just asked me to find a way to flip Dialect - I reminded him we wouldn't be able to pre-boot all of the Metadata but he needs it anyway. Clearly that's against all our optimisation principles, yet "technically doable".

[gsmet]

Agreed. I should have been more precise.

As for the dialect, I have in mind to allow all (most of) the versions of a given dialect (most of them are a pile of classes on top of the others so we get the old versions anyway if we include the latest). I don't know if it could satisfy Emmanuel's requirement.

At least, I hope we will experiment with that soon.

[Sanne]

So Emmanuel's requirement is to support multiple different DB vendors via the same native image. Requirement comes from Keycloak.

The only thing I can think of is to bake in each possible metadata, and then pick one at runtime to build the final SessionFactory. I guess we'll discuss it on its own issue soon, it's not on the top of my TODO list :)

[Sanne]

@gsmet should we close this one and create more specific issues?

I don't plan to make most options "runtime", that goes against both feasibility and optimisations, but I guess we'd need to discuss specific options and the impact they would have.

[nelsongraca]

While I agree with @Sanne that making everytthing runtime will go against optimizations, there are some that make sense to be runtime namely:
quarkus.hibernate-orm.database.default-schema
quarkus.hibernate-orm.database.default-catalog

I'm currently planning on moving some applications from Thorntail to Quarkus, but the default-schema is not the same in the multiple environments (production, QA, etc), and making it the same can't be done (out of our control).
Also our base principle is to use the EXACT same built artifacts for all the environments, building different artifacts for each environment has given me too many headaches in the past in other projects. And I'm not willing to go back to that kind of management.

I can try to provide the changes for these two, though I'm not very familiar with the codebase yet.

[Sanne]

hi @nelsongraca yes I totally agree with you. Those attributes deserve to become runtime; however - while I didn't check these specifically - I suspect that considering how the ORM bootstrap works today, this is really not a simple change.

Could you create a separate issue for these two? I'd like to spin-off individual issues rather than collecting them all in this one.

Also, feel free to try but be prepared for an uphill battle -- worse case, I'd be happy to reuse integration tests you might create!

[nelsongraca]

@Sanne Just created the issue, hopefully the information there is enough. I'm ready for an uphill battle, it may become more of a time issue, but I will try. No shame in failing.

[Sanne]

ok awesome that you're willing to investigate!

At high level, the problem is that we need to build the org.hibernate.boot.Metadata during the build phase (before runtime), and the Metadata instance will refer to a lot of internal state which represents the whole model of the application. Configuration attributes such as schema name are normally applied _before_ the Metadata is created, so it might have been picked up by a variety of other components.

It's not possible to rebuild the Metadata at runtime as it would be very complex to do correctly in native-image, but it might be possible to ensure the Metadata which was built before can be mutated.

So the difficulty of the task is in tracking down all the internal components in which one of such attributes is being consumed, and see if those can be reconfigured safely.

HTH

[famod]

This has bitten me badly today when I tried starting the runner ja with -Dquarkus.hibernate-orm.database.generation=....
What makes things worse is that there is no warning so users are pretty much left clueless.

@nelsongraca Have you made any progress on this? 🙂

[nelsongraca]

@famod Time has been short, started yesterday, still working on writing some tests.

[famod]

@nelsongraca that's more than I expected! Thanks!

[doublefx]

Hi @nelsongraca
Have you managed to dedicate a bit of time to look at that since then?