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

HHH-8732 Upgraded Ehcache to 2.7.5 #643

Closed
wants to merge 1 commit into from

Conversation

alexsnaps
Copy link
Contributor

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
Copy link

sarath commented Dec 10, 2013

the latest ehcache is 2.7.5, though.

@alexsnaps
Copy link
Contributor Author

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
Copy link
Member

brmeyer commented Jan 2, 2014

Hey @alexsnaps, any news on the version?

@alexsnaps
Copy link
Contributor Author

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

@brmeyer
Copy link
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
Copy link
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
Copy link
Contributor Author

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
Copy link
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
Copy link
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
Copy link
Contributor Author

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
Copy link
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
Copy link
Member

brmeyer commented Mar 19, 2014

Merging. Thanks much for the fix and discussion!

@Akansha1988
Copy link

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
Copy link
Contributor Author

@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 HHH-8732 branch November 10, 2015 09:30
@alexsnaps
Copy link
Contributor Author

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

@alexsnaps
Copy link
Contributor Author

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
Copy link

Thanks Alex!

@sebersole
Copy link
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

@alexsnaps
Copy link
Contributor Author

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
Copy link
Member

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

@alexsnaps
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

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

@alexsnaps
Copy link
Contributor Author

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

@sebersole
Copy link
Member

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

@sebersole sebersole closed this Nov 12, 2015
@Akansha1988
Copy link

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

@alexsnaps
Copy link
Contributor Author

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

@Akansha1988
Copy link

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

@alexsnaps
Copy link
Contributor Author

Dunno, @sebersole ?

@sebersole
Copy link
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)
.

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