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

ISPN-8982 Hibernate ORM 5.3 2LC implementation #5900

Merged
merged 1 commit into from Apr 30, 2018

Conversation

rvansa
Copy link
Member

@rvansa rvansa commented Apr 6, 2018

https://issues.jboss.org/browse/ISPN-8982

  • Includes infinispan-hibernate-cache-spi (and related changes) by Paul Ferraro
  • Regions moved from commons to version-specific modules.
  • infinispan-hibernate-cache is now merely a copy-paste of v51 with
    necessary SPI changes, and this module won't be in Infinispan 9.3
  • v53 drops support for transactional caches completely.

This is marked as preview for two reasons:

  1. Hibernate 5.3 used here is a SNAPSHOT version
  2. The PR should land in Infinispan 9.3 without the 5.2 support; only 9.2 should get it
    Despite of these I request review.

Copy link
Member

@galderz galderz left a comment

Choose a reason for hiding this comment

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

Looks good overall. I just had a question about cache-spi module, what's the role/objective of this module?

@@ -11,14 +11,23 @@ This chapter focuses on what you need to know to use Infinispan as second-level
Applications running in environments where Infinispan is not default cache provider for Hibernate will need to depend on the correct cache provider version.
The Infinispan cache provider version suitable for your application depends on the Hibernate version in use:

If using Hibernate 5.2 or higher, the Maven coordinates are the following:
If using Hibernate 5.3 or higher, the Maven coordinates are the following:
Copy link
Member

Choose a reason for hiding this comment

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

I guess it should say:

If using Hibernate 5.3, the Maven coordinates are the following:

Once we release higher versions we'll have new -vXX projects?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right.

I think that -vXX sets the expectations more clear. I could have moved 5.2 support from main to v52 packages but moving stuff around just because we have a newer version seems wrong to me. This approach should not break users who use FQCN for region factory.

@galderz
Copy link
Member

galderz commented Apr 9, 2018

Needs rebasing btw

@pferraro
Copy link
Member

pferraro commented Apr 9, 2018

@galderz The intention of the infinispan-hibernate-cache-spi is to decouple the application server 2LC integration from any specific version of Hibernate ORM.

@rvansa
Copy link
Member Author

rvansa commented Apr 12, 2018

Err, I have changed the way comparator is created for NSRW in v53 but it's wrong - the comparator in VersionedCallInterceptor must be able to compare all types stored in the region, not just the find which gets the AcessStrategy created. (this is a TODO for myself).

@rvansa
Copy link
Member Author

rvansa commented Apr 13, 2018

OK, I've backported some of the work from ISPN-9059 to this PR, letting VersionedCallInterceptor to fetch comparator for given type. I am not sure if NSRW access to a collection using structured entries (hibernate.cache.use_structured_entries=true) will work well but at the worst it should not cache anything. And IMO this would be a rare case.

@scottmarlow
Copy link
Member

Radim, when could a backport + release, of this change in 9.2.x be ready to include in WF13? I need to ask when it is too late to add a 9.2.x update into WF13, it would be helpful if I have an estimate of when this will be ready. Of course, I will report back with what I hear about when its too late. Thanks!

@rvansa
Copy link
Member Author

rvansa commented Apr 25, 2018

@scottmarlow We'll address the backport + release with maximum priority. I am just waiting for ORM release to change dependencies to non-snapshot version. Once that's out, I'll update & prepare 9.2 backport.

* Includes infinispan-hibernate-cache-spi (and related changes) by Paul Ferraro
* Regions moved from commons to version-specific modules.
* infinispan-hibernate-cache is now merely a copy-paste of v51 with
necessary SPI changes, and this module won't be in Infinispan 9.3
* v53 drops support for transactional caches completely.
@rvansa
Copy link
Member Author

rvansa commented Apr 27, 2018

Updated to use ORM CR2 & removed 5.2 compatibility (this is kept in #5946 only).

@galderz
Copy link
Member

galderz commented Apr 28, 2018

Thanks @rvansa! Can we integrate it now? Or is there anything else that needs doing?

@rvansa
Copy link
Member Author

rvansa commented Apr 30, 2018

@galderz There's no more work on this adapting the SPIs, everything is implemented. Improvements and optimizations will come as https://issues.jboss.org/browse/ISPN-9059 and https://issues.jboss.org/browse/ISPN-9075 later.

@galderz galderz merged commit ed091bd into infinispan:master Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants