9:13 a.m.
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.
Unfortunately, the EhCache documentation
<http://www.ehcache.org/documentation/2.8/integrations/hibernate.html#read...
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 <sanne(a)hibernate.org> wrote:
On 5 April 2016 at 14:11, Vlad Mihalcea
<mihalcea.vlad(a)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-cachecon...
>
> 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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev