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

Upgrade to Hibernate ORM 6.2 #31235

Merged
merged 83 commits into from
Feb 23, 2023
Merged

Upgrade to Hibernate ORM 6.2 #31235

merged 83 commits into from
Feb 23, 2023

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Feb 16, 2023

Upgrades the project to use Hibernate ORM 6 - at cost of not having a matching Hibernate Reactive at the moment.

Hibernate Reactive is making progress quickly towards 2.0, which is meant to be compatible with ORM 6.2, but it's not there yet; we need to harden and improve the ORM6 integration first so people will have to temporarily live w/o Hibernate Reactive - hopefully it won't take long to catch up.

Some more technical debt:

  • Hibernate Validation integration seems working fine, but it's logging nasty warnings (unnecessarily); this will be fixed when we upgrade to ORM 6.2.0.Final as the problem was in ORM core. Also have a POC for an alternative integration path which bypasses the issue and avoids some reflection needs - keeping this to the side as plan B.
  • Guides and documentation will need some love
  • Any help we might come up with to help people migrate would be welcome - I expect difficulties and the upstream ORM migration guide could use some love as well.

Some notes on the history: it's long but I've tried hard to keep my commits clean; in fact they have been rewritten and re-squashed multiple times so to be focused on atomic needs.

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/panache area/persistence OBSOLETE, DO NOT USE labels Feb 16, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 16, 2023

/cc @yrodiere (hibernate-orm)

@github-actions
Copy link

github-actions bot commented Feb 20, 2023

🙈 The PR is closed and the preview is expired.

@Sanne
Copy link
Member Author

Sanne commented Feb 20, 2023

Updated the description trying to capture the latest status.
N.B. it got push-forced over the weekend, if you got in trouble let me know I can help.

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to see this reach the main repo :)

I added a few comments following my first pass.

@Sanne Sanne changed the title Preview: upgrading to Hibernate ORM 6.2 Upgrading to Hibernate ORM 6.2 Feb 20, 2023
@Sanne
Copy link
Member Author

Sanne commented Feb 20, 2023

We got all integration tests working, and all comments addressed - marking as no longer a draft

@Sanne Sanne marked this pull request as ready for review February 20, 2023 21:10
@Sanne
Copy link
Member Author

Sanne commented Feb 21, 2023

native-image analysis stats

I suspect things will get worse if the application does something like sessionFactory.getSchemaManager().validateMappedObjects() since this will prevent GraalVM from eliminating the Metadata stored in the schema manager and add a whole lot of reachable code... Though it's an open question whether using SchemaManager in a production application is reasonable. I guess validation is somewhat reasonable, however exotic.

N.B. native image isn't going to prune unreachable data which we're referencing in the heap AFAIK; the dead code elimination applies to code only - possibly constants if they are stored in classes which don't get initialized.

So the metadata costs you're pointing to are most likely applicable even if the user doesn't have specific code triggering it.

@yrodiere
Copy link
Member

yrodiere commented Feb 21, 2023

N.B. native image isn't going to prune unreachable data which we're referencing in the heap AFAIK; the dead code elimination applies to code only - possibly constants if they are stored in classes which don't get initialized.

This still means it will prune unreachable code if there's no way this Metadata class is ever used, right? I.e. the native image will waste space for Metadata-related objects and their fields, as well as their classes, but without any methods?

Not sure there would be a big difference, though...

@Sanne
Copy link
Member Author

Sanne commented Feb 21, 2023

If an instance of a class X is present in the heap which gets captured in the image (just one reference to it is enough), then the whole class definition and all its methods is included as well - including all other symbols that might be reached from there.

So if Metadata has a method, even a private one, which you know is not going to be used at in the application, the compiler is often not smart enough to figure this out. And if that private method invokes a whole library Y, the reachable portion of Y will be compiled and included as well.

And that's why I've been working occasionally by inspecting heap dumps in Eclipse MAT :)

So what we need to do is to encode the knowledge that you might be able to infer from our code into a shape that the compiler can also nail it - without breaking semantics.

@quarkus-bot

This comment has been minimized.

@quarkus-bot
Copy link

quarkus-bot bot commented Feb 21, 2023

Failing Jobs - Building c23c65b

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

@gsmet gsmet changed the title Upgrading to Hibernate ORM 6.2 Upgrade to Hibernate ORM 6.2 Feb 23, 2023
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in and improve further in follow-up PRs.

@gsmet gsmet merged commit f36ae55 into quarkusio:main Feb 23, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Feb 23, 2023
@Sanne
Copy link
Member Author

Sanne commented Feb 23, 2023

Thanks @gsmet !
Also because I have some polishing work to follow - some of which is better handled with a dedicated review/discussion.

@rsvoboda
Copy link
Member

Switching to Hibernate 6 will be a big thing for Quarkus 3 users. Are you planning to list the key changes and link Hibernate migration guides in https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.0#jpa--hibernate-orm? Or there will be a dedicated page for Hibernate move?

@Sanne
Copy link
Member Author

Sanne commented Feb 24, 2023

Yes, we'll need to spend some time on documentation and migrations in particular for sure.
Personally I'll be primarily focused on catching up on the Hibernate Reactive side.

pjgg pushed a commit to pjgg/beefy-scenarios that referenced this pull request Feb 27, 2023
quarkusio/quarkus#31235 upgraded Hibernate to version 6.2. org.hibernate.annotations.ParamDef#type now accepts Class instead of plain string.
Also hibernate sequence generator needs some enhances and define the increment values on the migration SQL scripts
pjgg pushed a commit to pjgg/beefy-scenarios that referenced this pull request Feb 27, 2023
quarkusio/quarkus#31235 upgraded Hibernate to version 6.2. org.hibernate.annotations.ParamDef#type now accepts Class instead of plain string.
Also hibernate sequence generator needs some enhances and define the increment values on the migration SQL scripts
@rsvoboda
Copy link
Member

@Sanne who is the go-to person when you are focused on Hibernate Reactive? We need somebody who will look into migration troubles and help people to adjust their apps and deployments the right way.

Are there any docs that could be used as the starting point right now?

@Sanne
Copy link
Member Author

Sanne commented Feb 28, 2023

Hi @rsvoboda; @DavideD and @gavinking are the main experts for Hibernate Reactive - I'd say it's early days though, we haven't released a first Alpha of Hibernate Reactive 2.0 yet and there isn't a migration guide yet.

In many ways I expect the migration to Hibernate Reactive 2.0 actually to be simpler than the Hibernate ORM 6.2 one: other than it being now based on Jakarta, the APIs we expose so far haven't changed at all. It does inherit the changes in mapping and configuration properties that impacted ORM though.

@rsvoboda
Copy link
Member

@Sanne I was thinking about Hibernate ORM 6.2 in my previous comment.

@Sanne
Copy link
Member Author

Sanne commented Feb 28, 2023

@Sanne I was thinking about Hibernate ORM 6.2 in my previous comment.

I'm confused now. You specifically asked about Hibernate Reactive ?

But yes the links you found are the relevant ones, for both ORM "classic" and Reactive. But don't take them as final documents, they are live wikis and they're evolving as we speak - especially as community members try, when we find discrepencies it's being updated.

From the Quarkus side, it's TBD. I don't think we'll want to replicate all the content from the Hibernate wiki, but we might want to call out some particular sections which are more important than others. At very least we should link to it.

@rsvoboda
Copy link
Member

rsvoboda commented Mar 1, 2023

:) I asked who is the go-to person when you are focused on Hibernate Reactive? .. I meant go-to person for ORM topics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants