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

Fix Hibernate cache random failures #694 #710

Merged
merged 2 commits into from
Feb 1, 2019
Merged

Fix Hibernate cache random failures #694 #710

merged 2 commits into from
Feb 1, 2019

Conversation

galderz
Copy link
Member

@galderz galderz commented Feb 1, 2019

  • Caffeine offers no strict guarantees on which entity is evicted.
    Once the max is reached, there's no strict guarantees on which
    entity will be evicted, so we can't really to try to deterministically
    choose an entity to query after it's been evicted.
  • Caffeine is not strict in its max size, it can run over for a little.
  • Fix incorrect conversion from seconds to nanoseconds.

* Caffeine offers no strict guarantees on which entity is evicted.
Once the max is reached, there's no strict guarantees on which
entity will be evicted, so we can't really to try to deterministically
choose an entity to query after it's been evicted.
* Caffeine is not strict in its max size, it can run over for a little.
* Fix incorrect conversion from seconds to nanoseconds.
@galderz
Copy link
Member Author

galderz commented Feb 1, 2019

I'll wait until CI but I don't think this is fully fixed. Native image testing locally just threw:

Expected: is "OK"
  Actual: Oops, shit happened, No boot for you! java.lang.RuntimeException: [org.jboss.shamrock.example.infinispancachejpa.Person] expected Counts{puts=4, hits=0, misses=4, numElements=4} second level cache count, instead got: Counts{puts=0, hits=0, misses=4, numElements=0}
	java.lang.RuntimeException: [org.jboss.shamrock.example.infinispancachejpa.Person] expected Counts{puts=4, hits=0, misses=4, numElements=4} second level cache count, instead got: Counts{puts=0, hits=0, misses=4, numElements=0}
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.assertCountEquals(InfinispanCacheJPAFunctionalityTestEndpoint.java:735)
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.assertRegionStats(InfinispanCacheJPAFunctionalityTestEndpoint.java:730)
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.verifyFindByIdPersons(InfinispanCacheJPAFunctionalityTestEndpoint.java:463)
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.testReadOnly(InfinispanCacheJPAFunctionalityTestEndpoint.java:439)
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.doStuffWithHibernate(InfinispanCacheJPAFunctionalityTestEndpoint.java:61)
	at org.jboss.shamrock.example.infinispancachejpa.InfinispanCacheJPAFunctionalityTestEndpoint.doGet(InfinispanCacheJPAFunctionalityTestEndpoint.java:47)

@galderz
Copy link
Member Author

galderz commented Feb 1, 2019

I'll wait until CI finishes and then close the PR, I've found another issue :)

* PutFromLoadValidator compares expiration timeouts with region
  timestamps, which are provided in milliseconds.
* Caffeine works with nanoseconds, so max idle timeouts should be
  converted into nanoseconds.
* Empty VersionedEntry instances are tombstones, whose tomsbtone
  lifespan should be defined in nanoseconds so that Caffeine can evict
  it appropiately.
@galderz
Copy link
Member Author

galderz commented Feb 1, 2019

Added a new commit that should fix any pending failures. Essentially, different parts of the Hibernate cache logic work with different units. These units are mandated by Hibernate cache SPI and Caffeine respectively and crucially they're in different units. Hibernate cache SPI defines next region timestamp in milliseconds while Caffeine works with nanoseconds.

@Sanne
Copy link
Member

Sanne commented Feb 1, 2019

Looks nice, and worked locally. Waiting for CI to finish.

@dmlloyd
Copy link
Member

dmlloyd commented Feb 1, 2019

Maybe we want to get rid of the "shit happened" bit at some point :)

@galderz
Copy link
Member Author

galderz commented Feb 1, 2019

Maybe we want to get rid of the "shit happened" bit at some point :)

Copy pasted that from somewhere else... 🙈

@Sanne Sanne merged commit c494f1f into quarkusio:master Feb 1, 2019
@Sanne Sanne deleted the t_random_2lcache_failures branch February 1, 2019 14:03
@gsmet gsmet added this to the 0.8.0 milestone Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants