On 04/06/2016 11:58 AM, Radim Vansa wrote: > On 04/05/2016 04:13 PM, Vlad Mihalcea wrote: >> Hi, >> >> I'd definitely fix it for the refresh operation, which does an implicit >> cache eviction too. >> In this case, the proposed PR solution is fine since it simply locks the >> entry right after it is evicted from the cache and releases the lock after >> the transaction is ended. >> This way, we won't push an uncommitted entity into 2PL during the two-phase >> loading phase that is triggered by the refresh operation. >> >> For sessionFactory.getCache.evictEntity/evictCollection, if there is a >> current Session going on, we could propagate a >> CacheEvictEvent/CollectionCacheEvictEvent which can apply the lock on that >> particular entity/collection, and we release it right after the current >> transaction is ended, similar to what refresh should do as well. >> >> For every other use case, like evictAll/evictRegion, we should just >> document the behavior. >> >> I saw that Radim has added such a warning for Infinispan in the new User >> Guide: >> >> read-write mode is supported on non-transactional distributed/replicated >>> caches, however, eviction should not be used in this configuration. Use of >>> eviction can lead to consistency issues. > This is a different matter; in Infinispan 2LC impl you store locks and > entries either in two different caches (the entries cache is > invalidation, locks is local), or in single cache > (replicated/distributed). As we don't want to lose locks randomly, and > eviction picks entries unpredictably, its use is discouraged.
... it's use is discouraged in the repl/dist mode as you can't allow eviction on entries but prohibit on locks with both of these in single cache. > > I think that this issue does not apply to Infinispan with the > invalidation configuration, since evict/evictAll does not remove any > locks, and the lock blocks further updates (including putFromLoads) to > the entry in cache until the transaction commits. In case of > replicated/distributed cache, it seems that the evict is ignored after > update, but evictAll is not (that would qualify as a bug) - so after > evictAll you could observe the uncommitted read. Nevertheless I would > have to test this. > > Radim > >> Unfortunately, the EhCache documentation >> <http://www.ehcache.org/documentation/2.8/integrations/hibernate.html#read-write> >> makes a wrong assumption: >> >> read-write - Caches data that is sometimes updated while maintaining the >>> semantics of “read committed” isolation level. >> Vlad >> >> On Tue, Apr 5, 2016 at 4:30 PM, Sanne Grinovero <sa...@hibernate.org> wrote: >> >>> On 5 April 2016 at 14:11, Vlad Mihalcea <mihalcea.v...@gmail.com> wrote: >>>> Hi, >>>> >>>> While reviewing the PR for this issue: >>>> >>>> https://hibernate.atlassian.net/browse/HHH-10649 >>>> >>>> I realized that the ReadWrite cache concurrency strategy has a flaw that >>>> permits "read uncommitted" anomalies. >>>> The RW cache concurrency strategy guards any modifications with Lock >>>> entries, as explained in this post that I wrote some time ago: >>>> >>>> >>> http://vladmihalcea.com/2015/05/25/how-does-hibernate-read_write-cacheconcurrencystrategy-work/ >>>> Every time we update/delete an entry, a Lock is put in the cache under >>> the >>>> entity key, and, this way, "read uncommitted" anomalies should be >>> prevented. >>>> The problem comes when entries are evicted either explicitly: >>>> >>>> session.getSessionFactory().getCache().evictEntity( CacheableItem.class, >>>> item1.getId() ); >>>> >>>> or implicitly: >>>> >>>> session.refresh( item1 ); >>> Good catch! >>> >>> I think this is caused as we generally don't expect the evict >>> operation to be controlled explicitly. >>> In my personal experience, I would use the evictAll method to nuke the >>> cache state after some significant operation, like restoring a >>> backup.. and no other Session would have been opened in the meantime. >>> I never used an explicit single-shot evict so I can't say what the use >>> case would be. >>> >>> But of course you're right that it might be used differently, or at >>> least such a limitation should be clear. >>> >>>> During eviction, the 2PL will remove the Lock entry, and if the user >>>> attempts to load the entity anew (in the same transaction that has modified >>>> the entity but which is not committed yet), an uncommitted change could be >>>> propagated to the 2PL. >>>> >>>> This issue is replicated by the PR associated to this Jira issue, and I >>>> also replicated it with manual eviction and entity loading. >>>> >>>> To fix it, the RW cache concurrency strategy should not delete entries from >>>> 2PL upon eviction, but instead it should turn them in Lock entries. >>> I'm not sure I understood this part. Shouldn't it rather be allowed to >>> delete everything, except any existing locks? >>> Then rather than turn the remaining locks into locks, it would be >>> enough to leave them. >>> >>>> For the evict method, this is not really a problem, but evictAll would >>>> imply taking all entries and replacing them with Locks, and that might not >>>> perform very well in a distributed-cache scenario. >>>> >>>> Ideally, lock entries would be stored separately than actual cached value >>>> entries, and this problem would be fixed in a much cleaner fashion. >>> I'd leave this as a detail to the Cache implementation, some might be >>> able to perform some operation more efficiently. >>> Probably a good idea to clarify this expectation on the javadocs of >>> the SPI methods. >>> >>> Thanks, >>> Sanne >>> >>> >>>> Let me know what you think about this. >>>> >>>> Vlad >>>> _______________________________________________ >>>> hibernate-dev mailing list >>>> hibernate-dev@lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > -- Radim Vansa <rva...@redhat.com> JBoss Performance Team _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
