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

HHH-8732 Upgraded Ehcache to 2.7.5 #643

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@alexsnaps
Contributor

alexsnaps commented Dec 10, 2013

This not only addresses HHH-8732, but actually upgrades the ehcache dependency to a newer stable release, 2.7.4, which isn't compatible (compile & runtime) with older releases (new stats API & some config changes, as covered by HHH-8732)

@sarath

This comment has been minimized.

Show comment
Hide comment
@sarath

sarath Dec 10, 2013

the latest ehcache is 2.7.5, though.

sarath commented Dec 10, 2013

the latest ehcache is 2.7.5, though.

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Dec 10, 2013

Contributor

Is it ?! Hehe! Kinda scary I don't know that !!! I guess... working on 2.9.0, I have an excuse ;)
Just double checking, interestingly you are correct, but 2.7.5 doesn't seem to be on maven central somehow... Double checking internally what's going on.

Contributor

alexsnaps commented Dec 10, 2013

Is it ?! Hehe! Kinda scary I don't know that !!! I guess... working on 2.9.0, I have an excuse ;)
Just double checking, interestingly you are correct, but 2.7.5 doesn't seem to be on maven central somehow... Double checking internally what's going on.

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Jan 2, 2014

Member

Hey @alexsnaps, any news on the version?

Member

brmeyer commented Jan 2, 2014

Hey @alexsnaps, any news on the version?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Mar 12, 2014

Contributor

I've updated to 2.7.5
I hope this can make it to 4.3.0

Contributor

alexsnaps commented Mar 12, 2014

I've updated to 2.7.5
I hope this can make it to 4.3.0

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 12, 2014

Member

@alexsnaps, thanks much! Would this make hibernate-ehcache incompatable with users still explicitly using an older ehcache version? Wondering if we should hold off for ORM 5

Member

brmeyer commented Mar 12, 2014

@alexsnaps, thanks much! Would this make hibernate-ehcache incompatable with users still explicitly using an older ehcache version? Wondering if we should hold off for ORM 5

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 12, 2014

Member

Is there something we could do to address HHH-8732 without upgrading, then defer the actual upgrade until 5?

Member

brmeyer commented Mar 12, 2014

Is there something we could do to address HHH-8732 without upgrading, then defer the actual upgrade until 5?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Mar 14, 2014

Contributor

I fear the answer is no... While the initial issue fix is "minimal", these newer version of Ehcache use a new statistics framework and expose the stats differently now... Something that Hibernate depends on.
All these changes happened at once, with Ehcache 2.7. I think the 2.7 branch is mature enough and isn't too much of a risk.
For users that use Ehcache, that's generally the version used (and 2.8 is compatible). I think keeping 4.2< with the Ehcache 2.6 and 4.3 onwards is 2.7 is sensible. But not my call to make.

Contributor

alexsnaps commented Mar 14, 2014

I fear the answer is no... While the initial issue fix is "minimal", these newer version of Ehcache use a new statistics framework and expose the stats differently now... Something that Hibernate depends on.
All these changes happened at once, with Ehcache 2.7. I think the 2.7 branch is mature enough and isn't too much of a risk.
For users that use Ehcache, that's generally the version used (and 2.8 is compatible). I think keeping 4.2< with the Ehcache 2.6 and 4.3 onwards is 2.7 is sensible. But not my call to make.

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 19, 2014

Member

@alexsnaps, thanks much for the info! Since 4.3 is now our stable branch and should only be getting bug fixes, I'm inclined to pull this solely into 5. Any arguments against that?

Member

brmeyer commented Mar 19, 2014

@alexsnaps, thanks much for the info! Since 4.3 is now our stable branch and should only be getting bug fixes, I'm inclined to pull this solely into 5. Any arguments against that?

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 19, 2014

Member

And @alexsnaps, if the above is the case, is there anything we can do to correct HHH-8732 without upgrading the version? Simply remove ValueMode usage from HibernateUtil?

Member

brmeyer commented Mar 19, 2014

And @alexsnaps, if the above is the case, is there anything we can do to correct HHH-8732 without upgrading the version? Simply remove ValueMode usage from HibernateUtil?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Mar 19, 2014

Contributor

That would actually be really bad. Would break use cases for users not properly configuring their caches and using older versions, while fixing "newer" versions (that users would need to override the transitive dependency for)... Honestly, I don't think there is much we can do and =<4.3 would be Ehcache =<2.6 and 5 would then be newer version. The other option is to start doing so evil stuff with multiple code paths for either versions, but this would create a test & dep nightmare (the one magic I don't miss from the byte code voodoo we used to do).
What I could do though, is create an "alternate module" for people really wanting to use the 2.7+ ehcache lines with the 4.x line of Hibernate. So I'd say let's keep things as is for 4.x.
So I guess I will need to do a new pull request for the v5 then? Also, I should probably bring it up on the dev-ml, but any thoughts on migrating the cache api dependency to jsr107?

Contributor

alexsnaps commented Mar 19, 2014

That would actually be really bad. Would break use cases for users not properly configuring their caches and using older versions, while fixing "newer" versions (that users would need to override the transitive dependency for)... Honestly, I don't think there is much we can do and =<4.3 would be Ehcache =<2.6 and 5 would then be newer version. The other option is to start doing so evil stuff with multiple code paths for either versions, but this would create a test & dep nightmare (the one magic I don't miss from the byte code voodoo we used to do).
What I could do though, is create an "alternate module" for people really wanting to use the 2.7+ ehcache lines with the 4.x line of Hibernate. So I'd say let's keep things as is for 4.x.
So I guess I will need to do a new pull request for the v5 then? Also, I should probably bring it up on the dev-ml, but any thoughts on migrating the cache api dependency to jsr107?

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 19, 2014

Member

@alexsnaps, understood -- good points. No need to change the PR. master == ORM 5

Definitely interested to hear more about jsr107. The mailing list would be a great start.

Member

brmeyer commented Mar 19, 2014

@alexsnaps, understood -- good points. No need to change the PR. master == ORM 5

Definitely interested to hear more about jsr107. The mailing list would be a great start.

@brmeyer

This comment has been minimized.

Show comment
Hide comment
@brmeyer

brmeyer Mar 19, 2014

Member

Merging. Thanks much for the fix and discussion!

Member

brmeyer commented Mar 19, 2014

Merging. Thanks much for the fix and discussion!

@brmeyer brmeyer closed this Mar 19, 2014

@alexsnaps alexsnaps deleted the alexsnaps:HHH-8732 branch Mar 20, 2014

@tarioch tarioch referenced this pull request Jan 15, 2015

Closed

Support EHCache 2.4/2.6? #39

@Akansha1988

This comment has been minimized.

Show comment
Hide comment
@Akansha1988

Akansha1988 Oct 20, 2015

I still see ValueMode as an issue with Hibernate 5.2.FINAL and Terracotta 4.3 enterprise verison. Is the fix released for ORM 5X?

Akansha1988 commented Oct 20, 2015

I still see ValueMode as an issue with Hibernate 5.2.FINAL and Terracotta 4.3 enterprise verison. Is the fix released for ORM 5X?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 2, 2015

Contributor

@Akansha1988 Really hard to help here, you may want to drop a complete report to the ehcache users google group here : https://groups.google.com/forum/#!categories/ehcache-users/ehcache-hibernate

Contributor

alexsnaps commented Nov 2, 2015

@Akansha1988 Really hard to help here, you may want to drop a complete report to the ehcache users google group here : https://groups.google.com/forum/#!categories/ehcache-users/ehcache-hibernate

@alexsnaps alexsnaps restored the alexsnaps:HHH-8732 branch Nov 10, 2015

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 10, 2015

Contributor

@Akansha1988 Looks like you're right. Something is odd here. Am addressing this as part of HHH-10124

Contributor

alexsnaps commented Nov 10, 2015

@Akansha1988 Looks like you're right. Something is odd here. Am addressing this as part of HHH-10124

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 10, 2015

Contributor

Investigating this a little further, looks like the commit got "lost" on master around the 4.3.0.Final release (way back when). I think at the time master was to be 5.0. Anyways, I guess all will make it in as part of 5.1

Contributor

alexsnaps commented Nov 10, 2015

Investigating this a little further, looks like the commit got "lost" on master around the 4.3.0.Final release (way back when). I think at the time master was to be 5.0. Anyways, I guess all will make it in as part of 5.1

@Akansha1988

This comment has been minimized.

Show comment
Hide comment
@Akansha1988

Akansha1988 Nov 10, 2015

Thanks Alex!

Akansha1988 commented Nov 10, 2015

Thanks Alex!

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 10, 2015

Member

@alexsnaps Is the commit you are talking about on the upstream hibernate-orm/metamodel branch? That was master for a while, and the plan was for it to be 5.0, but developing the planned changes there took way too long so we decided to move the work on master off to that branch I mentioned; some of the work from that branch got moved to the new master to form what actually got released as 5.0. Confusing I know... sorry

Member

sebersole commented Nov 10, 2015

@alexsnaps Is the commit you are talking about on the upstream hibernate-orm/metamodel branch? That was master for a while, and the plan was for it to be 5.0, but developing the planned changes there took way too long so we decided to move the work on master off to that branch I mentioned; some of the work from that branch got moved to the new master to form what actually got released as 5.0. Confusing I know... sorry

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 10, 2015

Contributor

Ah! That may explain. I'll search as soon as I can. This PR is the
cherry-picked change from my initial branch. If it is on that branch would
it then better be merged back from there instead?
On Tue, Nov 10, 2015 at 4:20 PM Steve Ebersole notifications@github.com
wrote:

@alexsnaps https://github.com/alexsnaps Is the commit you are talking
about on the upstream hibernate-orm/metamodel branch? That was master for a
while, and the plan was for it to be 5.0, but developing the planned
changes there took way too long so we decided to move the work on master
off to that branch I mentioned; some of the work from that branch got moved
to the new master to form what actually got released as 5.0. Confusing I
know... sorry


Reply to this email directly or view it on GitHub
#643 (comment)
.

Contributor

alexsnaps commented Nov 10, 2015

Ah! That may explain. I'll search as soon as I can. This PR is the
cherry-picked change from my initial branch. If it is on that branch would
it then better be merged back from there instead?
On Tue, Nov 10, 2015 at 4:20 PM Steve Ebersole notifications@github.com
wrote:

@alexsnaps https://github.com/alexsnaps Is the commit you are talking
about on the upstream hibernate-orm/metamodel branch? That was master for a
while, and the plan was for it to be 5.0, but developing the planned
changes there took way too long so we decided to move the work on master
off to that branch I mentioned; some of the work from that branch got moved
to the new master to form what actually got released as 5.0. Confusing I
know... sorry


Reply to this email directly or view it on GitHub
#643 (comment)
.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 10, 2015

Member

Its up to you. Not sure how likely it is to apply cleanly.

Member

sebersole commented Nov 10, 2015

Its up to you. Not sure how likely it is to apply cleanly.

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 12, 2015

Contributor

It's on that branch. I don't think we can get this over in any sane way
though... or not that I know.
So this PR is probably best. Effectively the exact same thing... other than
this now having a newer version.
I guess, if the PR is fine, with you, it could be merged when you see fit.
Alex

On Tue, Nov 10, 2015 at 4:29 PM Steve Ebersole notifications@github.com
wrote:

Its up to you. Not sure how likely it is to apply cleanly.


Reply to this email directly or view it on GitHub
#643 (comment)
.

Contributor

alexsnaps commented Nov 12, 2015

It's on that branch. I don't think we can get this over in any sane way
though... or not that I know.
So this PR is probably best. Effectively the exact same thing... other than
this now having a newer version.
I guess, if the PR is fine, with you, it could be merged when you see fit.
Alex

On Tue, Nov 10, 2015 at 4:29 PM Steve Ebersole notifications@github.com
wrote:

Its up to you. Not sure how likely it is to apply cleanly.


Reply to this email directly or view it on GitHub
#643 (comment)
.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 12, 2015

Member

Ok. I'll create a new Jira and link this PR to it and try to get it applied today.

Member

sebersole commented Nov 12, 2015

Ok. I'll create a new Jira and link this PR to it and try to get it applied today.

@sebersole sebersole reopened this Nov 12, 2015

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 12, 2015

Member

FWIW, GitHub at least seems to think this will not apply cleanly to master...

Member

sebersole commented Nov 12, 2015

FWIW, GitHub at least seems to think this will not apply cleanly to master...

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Nov 12, 2015

Contributor

Y, this one won't work, but #1142 should

Contributor

alexsnaps commented Nov 12, 2015

Y, this one won't work, but #1142 should

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 12, 2015

Member

I guess I misunderstood when you commented "the PR" on this PR ;)

Member

sebersole commented Nov 12, 2015

I guess I misunderstood when you commented "the PR" on this PR ;)

@sebersole sebersole closed this Nov 12, 2015

@Akansha1988

This comment has been minimized.

Show comment
Hide comment
@Akansha1988

Akansha1988 Dec 7, 2015

@alexsnaps Hi Alex, just wanted to check if there is any update on the fix?

Akansha1988 commented Dec 7, 2015

@alexsnaps Hi Alex, just wanted to check if there is any update on the fix?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Dec 7, 2015

Contributor

@Akansha1988 it got merged in as #1142 that'll be in 5.1 only though iirc...

Contributor

alexsnaps commented Dec 7, 2015

@Akansha1988 it got merged in as #1142 that'll be in 5.1 only though iirc...

@Akansha1988

This comment has been minimized.

Show comment
Hide comment
@Akansha1988

Akansha1988 Dec 7, 2015

@alexsnaps Could you please let me know when is 5.1 expected?

Akansha1988 commented Dec 7, 2015

@alexsnaps Could you please let me know when is 5.1 expected?

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps Dec 7, 2015

Contributor

Dunno, @sebersole ?

Contributor

alexsnaps commented Dec 7, 2015

Dunno, @sebersole ?

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Dec 8, 2015

Member

Sometime after the holidays. Not sure yet exactly.

On Mon, Dec 7, 2015, 3:34 PM Alex Snaps notifications@github.com wrote:

Dunno, @sebersole https://github.com/sebersole ?


Reply to this email directly or view it on GitHub
#643 (comment)
.

Member

sebersole commented Dec 8, 2015

Sometime after the holidays. Not sure yet exactly.

On Mon, Dec 7, 2015, 3:34 PM Alex Snaps notifications@github.com wrote:

Dunno, @sebersole https://github.com/sebersole ?


Reply to this email directly or view it on GitHub
#643 (comment)
.

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