Introduce Infinispan as cache and hibernate back-end #577
Conversation
@seanf Sean, do you mind taking an early look? No configuration details in the PR, I'll create a wiki page for those. |
9d4736e
to
f2ca245
Compare
For configuration details (still a very early draft) see https://github.com/zanata/zanata-server/wiki/Infinispan |
8c1a38e
to
3a3d8ee
Compare
@carlosmunoz Shouldn't we keep the integration tests' configuration as close to production as we can, in terms of persistence? |
Might not be a bad idea for the arquillian tests. I'm sure the JdbcStore works with H2. Until we get some serious performance tests I don't think we'll be able to see anything go to the database though, but at least it will validate our configuration. |
|
||
@Override | ||
public V getWithLoader(K key) { | ||
// NB: Need to manually implement the cache loader feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this method be synchronised or something?
What are the effects of switching from CacheConcurrencyStrategy.READ_WRITE to CacheConcurrencyStrategy.TRANSACTIONAL? |
cacheManager.shutdown(); | ||
// NB Since infinispan is container managed, there's no need to stop the | ||
// cache manager with | ||
// cacheContainer.stop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well get rid of the whole method then. One less thing to migrate from Seam to CDI.
From what I've investigated, READ_WRITE is the least performant of all the basic options just because it locks entries and compares versions (hash?) of objects when saving. TRANSACTIONAL is the one designed specifically for distributed caches. It does not lock, but rather relies on the cache to provide consistency. It can be registered with the local JTA transaction and comes with the price of XA. So, it will be less efficient, but if we can use the option to save in the background it might not be noticeable from a response time perspective... might even be faster than our current solution. |
@Override | ||
protected void prepareResources() { | ||
MockitoAnnotations.initMocks(this); | ||
|
||
cacheContainer.start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we call cacheContainer.stop()
somewhere too?
I won't list all of them, but I think all the tests which |
value="infinispan" /> | ||
<!-- to use an existing cache manager (Good for external configuration) --> | ||
<property name="hibernate.search.infinispan.cachemanager_jndiname" | ||
value="java:jboss/infinispan/container/hibernate-search"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this bit (from the other persistence.xml)?
<property name="hibernate.cache.region.factory_class"
value="org.hibernate.cache.infinispan.JndiInfinispanRegionFactory" />
<property name="hibernate.cache.infinispan.cachemanager"
value="java:jboss/infinispan/container/hibernate" />
dcd7aa2
to
671603b
Compare
Conflicts: zanata-war/src/test/java/org/zanata/service/impl/StatisticsServiceImplTest.java
647ac21
to
0501b44
Compare
@seanf Can we consider merging this into master now that we've branched for release? |
@seanf bump |
@carlosmunoz Looks good, but we might need QA to look again, after the branch is rebased (or master is merged in). |
Conflicts: pom.xml zanata-war/pom.xml zanata-war/src/main/java/org/zanata/service/impl/TranslationStateCacheImpl.java
2807aef
to
8dc9f6c
Compare
@seanf Care to review the last commits for this pull request? |
<h5>Infrastructure Changes</h5> | ||
|
||
Zanata now uses Infinispan as its cache provider, and the cache needs to be configured in Jboss' `standalone.xml` file. Please see the [Infinispan](configuration/infinispan) section for more information. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this pull request updates all SIX! copies of standalone*.xml in the source tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the configuration to all our standalone.xml files.
Otherwise looks fine. |
No. We left hibernate search out of infinispan for the time being due to less than desirable performance. |
👍 Once the tests pass. I would suggest running all functional tests before merging though. |
Introduce Infinispan as cache and hibernate back-end Full functional tests passed. Not attempting a squash merge.
Make infinispan the backend used for all caches (hibernate included) as well as hibernate search indexes. All configuration for caching now falls on standalone.xml. Additionally, these caches can be made to persist to the database meaning it's now possible to have caches that persist server shutdowns (statistics come to mind here), as well as keep them solely in memory (to test hibernate search without needing to create index files)