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-11266 Spring Cache 2nd-level cache #1639

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@candrews
Contributor

candrews commented Nov 21, 2016

Provides support for Spring Cache as a Hibernate 2nd level cache by adapting Spring Cache to jcache and building on the work done in https://hibernate.atlassian.net/browse/HHH-10770

I'm looking forward to feedback and guidance - thank you!

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 21, 2016

Member

If Spring Cache is adapted to the JCache contracts then I am not understanding the purpose of a whole new module in Hibernate. Users would simply depend on (1) hibernate-jcache and (2) the needed Spring dependencies and it would simply work.

Or are you saying that Spring Cache does not implement JCache, but rather your hibernate-spring-cache module wraps it as a JCache?

Part of the reason for wanting a JCache provider was to get out of the business of having to maintain all these individual caching integrations.

Member

sebersole commented Nov 21, 2016

If Spring Cache is adapted to the JCache contracts then I am not understanding the purpose of a whole new module in Hibernate. Users would simply depend on (1) hibernate-jcache and (2) the needed Spring dependencies and it would simply work.

Or are you saying that Spring Cache does not implement JCache, but rather your hibernate-spring-cache module wraps it as a JCache?

Part of the reason for wanting a JCache provider was to get out of the business of having to maintain all these individual caching integrations.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 21, 2016

Member

Looking through the actual changes it would seem the latter - that Spring Cache does not implement JCache contracts and that your hibernate-spring-cache module is intended to be the adapter.

Like I said, we want to get out of the business of maintaining these cache providers in Hibernate but I will start a discussion of this on the dev mailing list

Member

sebersole commented Nov 21, 2016

Looking through the actual changes it would seem the latter - that Spring Cache does not implement JCache contracts and that your hibernate-spring-cache module is intended to be the adapter.

Like I said, we want to get out of the business of maintaining these cache providers in Hibernate but I will start a discussion of this on the dev mailing list

@candrews

This comment has been minimized.

Show comment
Hide comment
@candrews

candrews Nov 21, 2016

Contributor

that Spring Cache does not implement JCache contracts and that your hibernate-spring-cache module is intended to be the adapter

That's right.

Currently, the situation is that a user has to configure a cache in Spring (to use with, for example, declarative caching / @Cache) then configure that same cache using a totally different system (different configuration files, different syntax, and probably different implementation) in Hibernate. For example, if a user is using Spring Cloud AWS's Elasticache support, then there's no way to connect Hibernate into that support. One would have to get a different library, setup totally new configuration files, and then make sure everything aligns. Even then, it's a bit weird that Hibernate and Spring would use different (usually TCP) connections to the cache server(s), which would impact scalability, consistency, and other such things.

With hibernate-spring-cache, Hibernate would use Spring's Cache, and everything Just Works with no duplication in configuration and no additional libraries.

I will start a discussion of this on the dev mailing list

For future visitors, here's that thread: https://lists.jboss.org/pipermail/hibernate-dev/2016-November/015596.html

Contributor

candrews commented Nov 21, 2016

that Spring Cache does not implement JCache contracts and that your hibernate-spring-cache module is intended to be the adapter

That's right.

Currently, the situation is that a user has to configure a cache in Spring (to use with, for example, declarative caching / @Cache) then configure that same cache using a totally different system (different configuration files, different syntax, and probably different implementation) in Hibernate. For example, if a user is using Spring Cloud AWS's Elasticache support, then there's no way to connect Hibernate into that support. One would have to get a different library, setup totally new configuration files, and then make sure everything aligns. Even then, it's a bit weird that Hibernate and Spring would use different (usually TCP) connections to the cache server(s), which would impact scalability, consistency, and other such things.

With hibernate-spring-cache, Hibernate would use Spring's Cache, and everything Just Works with no duplication in configuration and no additional libraries.

I will start a discussion of this on the dev mailing list

For future visitors, here's that thread: https://lists.jboss.org/pipermail/hibernate-dev/2016-November/015596.html

@snicoll

This comment has been minimized.

Show comment
Hide comment
@snicoll

snicoll Nov 22, 2016

@sebersole Spring Framework is not a cache library so it shouldn't come as a surprise it doesn't implement JCache. Hibernate doesn't either (it adapts to it) so we're pretty much at the same level here.

Part of the reason for wanting a JCache provider was to get out of the business of having to maintain all these individual caching integrations.

I can definitely relate to that and I think that's a good call indeed. What I like about @candrews proposal is that you'd get access to a wider range of options for Spring-based apps. There's a lifecycle issue to be solved obviously (I haven't looked at the code yet) but the cache contract is very mature and we have plenty of implementations out there. If you decide to give that a shot, I am more than happy to help.

snicoll commented Nov 22, 2016

@sebersole Spring Framework is not a cache library so it shouldn't come as a surprise it doesn't implement JCache. Hibernate doesn't either (it adapts to it) so we're pretty much at the same level here.

Part of the reason for wanting a JCache provider was to get out of the business of having to maintain all these individual caching integrations.

I can definitely relate to that and I think that's a good call indeed. What I like about @candrews proposal is that you'd get access to a wider range of options for Spring-based apps. There's a lifecycle issue to be solved obviously (I haven't looked at the code yet) but the cache contract is very mature and we have plenty of implementations out there. If you decide to give that a shot, I am more than happy to help.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 22, 2016

Member

So let me ask the obvious question... Why not just have Spring Cache implement JCache contracts?

Member

sebersole commented Nov 22, 2016

So let me ask the obvious question... Why not just have Spring Cache implement JCache contracts?

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 22, 2016

Member

@snicoll I understand that Spring is not a cache implementor per-se. But it does have a cache API/SPI: org.springframework.cache.CacheManager. So I was asking why org.springframework.cache.CacheManager can't implement javax.cache.CacheManager. That seems honestly the best solution

Member

sebersole commented Nov 22, 2016

@snicoll I understand that Spring is not a cache implementor per-se. But it does have a cache API/SPI: org.springframework.cache.CacheManager. So I was asking why org.springframework.cache.CacheManager can't implement javax.cache.CacheManager. That seems honestly the best solution

@snicoll

This comment has been minimized.

Show comment
Hide comment
@snicoll

snicoll Nov 22, 2016

So let me ask the obvious question... Why not just have Spring Cache implement JCache contracts?

Because we're not a cache library? I mean, do you realize that if we did that:

  1. All the third party implementations of our CacheManager would have to be updated to reflect the new contract
  2. Our contract is simple by design to allow as many implementations as possible. That wouldn't be the case anymore (if there was a way to have a generic JSR-107 implementation on something else, I am sure someone else would have discovered that by now)

That's why our JCache support is a simple adapter on our contract. The most difficult part was to handling the standard annotations which, arguably, is the thing that we should focus on.

Look simply at getCachingProvider. What do you think our implementation should return there exactly? It's not the full spec implementation but it's damn close IMO.

snicoll commented Nov 22, 2016

So let me ask the obvious question... Why not just have Spring Cache implement JCache contracts?

Because we're not a cache library? I mean, do you realize that if we did that:

  1. All the third party implementations of our CacheManager would have to be updated to reflect the new contract
  2. Our contract is simple by design to allow as many implementations as possible. That wouldn't be the case anymore (if there was a way to have a generic JSR-107 implementation on something else, I am sure someone else would have discovered that by now)

That's why our JCache support is a simple adapter on our contract. The most difficult part was to handling the standard annotations which, arguably, is the thing that we should focus on.

Look simply at getCachingProvider. What do you think our implementation should return there exactly? It's not the full spec implementation but it's damn close IMO.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 22, 2016

Member

Because we're not a cache library?

But Hibernate is? ;)

Member

sebersole commented Nov 22, 2016

Because we're not a cache library?

But Hibernate is? ;)

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 22, 2016

Member

Anyway, its clear Spring Cache will not be implementing JCache, so we will consider this under that knowledge.

Member

sebersole commented Nov 22, 2016

Anyway, its clear Spring Cache will not be implementing JCache, so we will consider this under that knowledge.

@Sanne

This comment has been minimized.

Show comment
Hide comment
@Sanne

Sanne Nov 22, 2016

Member

Wouldn't it be useful for Spring users to have a wrapper which can decorate a Spring Cache implementor to have it implement the standard API?
That way you could plug it into Hibernate using the JCache integration point, but also others could benefit from it.

Member

Sanne commented Nov 22, 2016

Wouldn't it be useful for Spring users to have a wrapper which can decorate a Spring Cache implementor to have it implement the standard API?
That way you could plug it into Hibernate using the JCache integration point, but also others could benefit from it.

@snicoll

This comment has been minimized.

Show comment
Hide comment
@snicoll

snicoll Nov 22, 2016

We're going in circles here Steve. My first comment mentioned very clearly that we're not a cache library and Hibernate isn't either.

@Sanne that's essentially what Steve is asking us to do. If you look at the spec and what it takes to implement those interfaces you'll quickly realize there are a bunch of things we'll have to implement that we shouldn't. My whole point is that it can't be a "wrapper" once you need to provide a CachingProvider. We're adapting to cache libraries, we're not implementing one. Hibernate should do the exact same thing IMO.

snicoll commented Nov 22, 2016

We're going in circles here Steve. My first comment mentioned very clearly that we're not a cache library and Hibernate isn't either.

@Sanne that's essentially what Steve is asking us to do. If you look at the spec and what it takes to implement those interfaces you'll quickly realize there are a bunch of things we'll have to implement that we shouldn't. My whole point is that it can't be a "wrapper" once you need to provide a CachingProvider. We're adapting to cache libraries, we're not implementing one. Hibernate should do the exact same thing IMO.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Nov 22, 2016

Member

Hibernate does do that. We just made the conscious decision to limit the cache backends we maintain integration for, because we are not the experts on those backends.

Member

sebersole commented Nov 22, 2016

Hibernate does do that. We just made the conscious decision to limit the cache backends we maintain integration for, because we are not the experts on those backends.

@Sanne

This comment has been minimized.

Show comment
Hide comment
@Sanne

Sanne Nov 22, 2016

Member

I see Stéphane, thanks that makes sense.

On 22 Nov 2016 5:37 p.m., "Stéphane Nicoll" notifications@github.com
wrote:

We're going in circles here Steve. My first comment mentioned very
clearly that we're not a cache library and Hibernate isn't either.

@Sanne that's essentially what Steve is asking us to do. If you look at
the spec and what it takes to implement those interfaces you'll quickly
realize there are a bunch of things we'll have to implement that we
shouldn't. My whole point is that it can't be a "wrapper" once you need to
provide a CachingProvider. We're adapting to cache libraries, we're not
implementing one. Hibernate should do the exact same thing IMO.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Member

Sanne commented Nov 22, 2016

I see Stéphane, thanks that makes sense.

On 22 Nov 2016 5:37 p.m., "Stéphane Nicoll" notifications@github.com
wrote:

We're going in circles here Steve. My first comment mentioned very
clearly that we're not a cache library and Hibernate isn't either.

@Sanne that's essentially what Steve is asking us to do. If you look at
the spec and what it takes to implement those interfaces you'll quickly
realize there are a bunch of things we'll have to implement that we
shouldn't. My whole point is that it can't be a "wrapper" once you need to
provide a CachingProvider. We're adapting to cache libraries, we're not
implementing one. Hibernate should do the exact same thing IMO.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@snicoll

This comment has been minimized.

Show comment
Hide comment
@snicoll

snicoll Nov 23, 2016

Hibernate does do that. We just made the conscious decision to limit the cache backends we maintain integration for, because we are not the experts on those backends.

I definitely can relate to that. I understand your goal is to limit to JSR-107 and expect your users to use a JCache-compliant caching library. My take here is that the Spring abstraction is quite simple and does not export any backend complexity (that's the whole purpose of the abstraction!). So if you decide to take JSR-107 and Spring, then you give a bunch of choices to your users (those who are on the JavaEE side will use JSR-107 anyway).

Does that sound crazy? Again, happy to help.

snicoll commented Nov 23, 2016

Hibernate does do that. We just made the conscious decision to limit the cache backends we maintain integration for, because we are not the experts on those backends.

I definitely can relate to that. I understand your goal is to limit to JSR-107 and expect your users to use a JCache-compliant caching library. My take here is that the Spring abstraction is quite simple and does not export any backend complexity (that's the whole purpose of the abstraction!). So if you decide to take JSR-107 and Spring, then you give a bunch of choices to your users (those who are on the JavaEE side will use JSR-107 anyway).

Does that sound crazy? Again, happy to help.

@candrews

This comment has been minimized.

Show comment
Hide comment
@candrews

candrews Dec 14, 2016

Contributor

So has a decision been reached?

If not, what can be done to facilitate coming to a decision?

Thanks again!

Contributor

candrews commented Dec 14, 2016

So has a decision been reached?

If not, what can be done to facilitate coming to a decision?

Thanks again!

@candrews

This comment has been minimized.

Show comment
Hide comment
@candrews
Contributor

candrews commented Jan 23, 2017

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Jan 25, 2017

Member

I've said all along I am basically -1 to this. I have been waiting for others to chime in. So far none have. Which tells me no one else is really interested in the discussion. So if you really want to push for an answer I can do that but you will probably not like the answer ;)

This is always the issue with these sort of integration artifacts. To me this should be a Spring module. To y'all it should be a Hibernate module. And as @snicoll points out we will continue to go around and around about it.

We all just happen to be at a face-to-face meeting this week. And Thursday we have some cache related discussions planned. I will again bring this up and see if anyone can convince me that we should do this. Either way I will have an final answer for this by end of week.

Member

sebersole commented Jan 25, 2017

I've said all along I am basically -1 to this. I have been waiting for others to chime in. So far none have. Which tells me no one else is really interested in the discussion. So if you really want to push for an answer I can do that but you will probably not like the answer ;)

This is always the issue with these sort of integration artifacts. To me this should be a Spring module. To y'all it should be a Hibernate module. And as @snicoll points out we will continue to go around and around about it.

We all just happen to be at a face-to-face meeting this week. And Thursday we have some cache related discussions planned. I will again bring this up and see if anyone can convince me that we should do this. Either way I will have an final answer for this by end of week.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Jan 25, 2017

Member

A few questions...

  1. Y'all have said that lots of Spring users use this caching. We'd like to understand this better. "Lots" means different things to different people.
  2. Would this be something usable by Hibernate users outside of Spring containers? I know y'all historically hate that term, but lets call a spade a spade ;)
  3. Is there a list (or could y'all enumerate) the different back ends Spring Cache supports?

I have discussed with the ORM tem and the consensus is to not accept this. I want to wait to get input from @Sanne who was not involved in the discussion I mentioned and answers to these questions would be the discussion points he and I would have.

Thanks!

Member

sebersole commented Jan 25, 2017

A few questions...

  1. Y'all have said that lots of Spring users use this caching. We'd like to understand this better. "Lots" means different things to different people.
  2. Would this be something usable by Hibernate users outside of Spring containers? I know y'all historically hate that term, but lets call a spade a spade ;)
  3. Is there a list (or could y'all enumerate) the different back ends Spring Cache supports?

I have discussed with the ORM tem and the consensus is to not accept this. I want to wait to get input from @Sanne who was not involved in the discussion I mentioned and answers to these questions would be the discussion points he and I would have.

Thanks!

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Jan 25, 2017

Member

Also, if we do decide to accept this we'd basically require that we get timely support from Spring Cache developers for any bugs/issues. We would take care of breakages from the Hibernate caching SPI, but anything else we would not touch. And if we are not getting timely support we would essentially drop this module.

Obviously "timely" is a relative term.

I know it sounds heavy handed, but we are trying to avoid cases (which we have had many in the past - hence our apprehension) of code dumping.

Member

sebersole commented Jan 25, 2017

Also, if we do decide to accept this we'd basically require that we get timely support from Spring Cache developers for any bugs/issues. We would take care of breakages from the Hibernate caching SPI, but anything else we would not touch. And if we are not getting timely support we would essentially drop this module.

Obviously "timely" is a relative term.

I know it sounds heavy handed, but we are trying to avoid cases (which we have had many in the past - hence our apprehension) of code dumping.

@candrews

This comment has been minimized.

Show comment
Hide comment
@candrews

candrews Jan 25, 2017

Contributor

That doesn't sound heavy handed at all - it sounds quite reasonable. Thanks for the update, I look forward to learning how your upcoming conversations go.

Contributor

candrews commented Jan 25, 2017

That doesn't sound heavy handed at all - it sounds quite reasonable. Thanks for the update, I look forward to learning how your upcoming conversations go.

HHH-11266 Spring Cache 2nd-level cache
Builds on hibernate-jcache
@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Feb 20, 2017

Member

@candrews Initially we had all decided to not accept this, but I am personally changing my mind about that for a number of reasons so let's see if we can convince the others. @dreab8, @Naros, et.al. what do you guys think? @Sanne you had some specific thoughts, care to share those here or on the associated Jira (https://hibernate.atlassian.net/browse/HHH-11266)?

One thing I would ask though @candrews is for you adapt this to the SPI changes being discussed for 6.0 https://hibernate.atlassian.net/browse/HHH-11356. The background and all the reasons we want to change it are discussed there. That design is still evolving as you will see from the Jira so please add any thoughts you have to the Jira.

Member

sebersole commented Feb 20, 2017

@candrews Initially we had all decided to not accept this, but I am personally changing my mind about that for a number of reasons so let's see if we can convince the others. @dreab8, @Naros, et.al. what do you guys think? @Sanne you had some specific thoughts, care to share those here or on the associated Jira (https://hibernate.atlassian.net/browse/HHH-11266)?

One thing I would ask though @candrews is for you adapt this to the SPI changes being discussed for 6.0 https://hibernate.atlassian.net/browse/HHH-11356. The background and all the reasons we want to change it are discussed there. That design is still evolving as you will see from the Jira so please add any thoughts you have to the Jira.

@candrews

This comment has been minimized.

Show comment
Hide comment
@candrews

candrews Feb 22, 2017

Contributor

I'll take a look at the 6.0 SPI changes and see what I can do.

In the mean time, I extracted the work from this PR into its own project: https://github.com/candrews/hibernate-springcache I've also published the artifacts to maven central for ease of use. I doubt many people will discover my project, and I think it's better to have this be part of Hibernate for other reasons well, so I hope we can eventually get it to the point of inclusion.

Thanks again!

Contributor

candrews commented Feb 22, 2017

I'll take a look at the 6.0 SPI changes and see what I can do.

In the mean time, I extracted the work from this PR into its own project: https://github.com/candrews/hibernate-springcache I've also published the artifacts to maven central for ease of use. I doubt many people will discover my project, and I think it's better to have this be part of Hibernate for other reasons well, so I hope we can eventually get it to the point of inclusion.

Thanks again!

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Mar 15, 2018

Member

@candrews one clarification. It was decided to instead release these SPI changes in 5.3. The old SPI design itself led to a few nasty bugs which we felt were important enough to warrant breaking the SPI.

@Sanne @dreab8 @Naros - one last ping. Thoughts? Or should we just reject this and move on? I still think a Spring module supporting this makes the most sense.

Member

sebersole commented Mar 15, 2018

@candrews one clarification. It was decided to instead release these SPI changes in 5.3. The old SPI design itself led to a few nasty bugs which we felt were important enough to warrant breaking the SPI.

@Sanne @dreab8 @Naros - one last ping. Thoughts? Or should we just reject this and move on? I still think a Spring module supporting this makes the most sense.

@dreab8

This comment has been minimized.

Show comment
Hide comment
@dreab8

dreab8 Mar 15, 2018

Member

@sebersole I'm not against integrating it but if and only if the maintaining burden will not be on the Hibernate Team.

Member

dreab8 commented Mar 15, 2018

@sebersole I'm not against integrating it but if and only if the maintaining burden will not be on the Hibernate Team.

@Naros

This comment has been minimized.

Show comment
Hide comment
@Naros

Naros Mar 15, 2018

Member

@sebersole @candrews I'm fine with integrating it as a hibernate sub-project as long as we have the understanding that its someone from the spring side who is responsible for maintaining it.

One thought is whether it makes sense to create a caching directory and relocate hibernate-jcache there along with this particular implementation?

Member

Naros commented Mar 15, 2018

@sebersole @candrews I'm fine with integrating it as a hibernate sub-project as long as we have the understanding that its someone from the spring side who is responsible for maintaining it.

One thought is whether it makes sense to create a caching directory and relocate hibernate-jcache there along with this particular implementation?

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Mar 15, 2018

Member

One thought is whether it makes sense to create a caching directory and relocate hibernate-jcache there along with this particular implementation?

That is an unrelated discussion and one that would apply equally to ConnectionProviders as well.

Member

sebersole commented Mar 15, 2018

One thought is whether it makes sense to create a caching directory and relocate hibernate-jcache there along with this particular implementation?

That is an unrelated discussion and one that would apply equally to ConnectionProviders as well.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Mar 15, 2018

Member

if and only if the maintaining burden will not be on the Hibernate Team.

we have the understanding that its someone from the spring side who is responsible for maintaining it.

In my experience these are things we should not necessarily expect. And even if the Spring team(s) do generally live up to this, we also have to keep in mind that they will have different priorities affecting how/when that is done. Take 5.3 and these caching SPI changes... the decision to introduce these in 5.3 rather than wait for 6.0 came about pretty quickly - could we really rely on an external contributor to work on that in the time-frame and with the priority we would need?

I still think having this as a Spring module makes the most sense[1], though they seem to feel the opposite.

[1] The reason I think this btw is a matter of dependencies. Spring already has a dependency on Hibernate to implement its Hibernate support. From that perspective it makes much more sense to add the support to Spring.

Member

sebersole commented Mar 15, 2018

if and only if the maintaining burden will not be on the Hibernate Team.

we have the understanding that its someone from the spring side who is responsible for maintaining it.

In my experience these are things we should not necessarily expect. And even if the Spring team(s) do generally live up to this, we also have to keep in mind that they will have different priorities affecting how/when that is done. Take 5.3 and these caching SPI changes... the decision to introduce these in 5.3 rather than wait for 6.0 came about pretty quickly - could we really rely on an external contributor to work on that in the time-frame and with the priority we would need?

I still think having this as a Spring module makes the most sense[1], though they seem to feel the opposite.

[1] The reason I think this btw is a matter of dependencies. Spring already has a dependency on Hibernate to implement its Hibernate support. From that perspective it makes much more sense to add the support to Spring.

@sebersole

This comment has been minimized.

Show comment
Hide comment
@sebersole

sebersole Mar 16, 2018

Member

Going to close this. I think this is a great thing to have, I just do not think it belongs as part of the Hibernate project. I am more than happy to help with implementing this or advising on its implementation fwiw.

Member

sebersole commented Mar 16, 2018

Going to close this. I think this is a great thing to have, I just do not think it belongs as part of the Hibernate project. I am more than happy to help with implementing this or advising on its implementation fwiw.

@sebersole sebersole closed this Mar 16, 2018

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