p.s. I'm yet to verify whether such a misbehaving key produce such NPE in 
Infinispan... could be the case that this is all caused by my integration 
efforts in 2LC...

Cheers,
--
Galder Zamarreño
Infinispan, Red Hat

> On 15 Feb 2017, at 18:07, Galder Zamarreño <gal...@redhat.com> wrote:
> 
> 
> --
> Galder Zamarreño
> Infinispan, Red Hat
> 
>> On 15 Feb 2017, at 12:21, Radim Vansa <rva...@redhat.com> wrote:
>> 
>> On 02/15/2017 11:28 AM, Galder Zamarreño wrote:
>>> Hey Radim,
>>> 
>>> Your changes in ISPN-7029 are causing issues with Hibernate 2LC.
>>> 
>>> In particular [2]. Name.equals() always returns false, so it'll never be 
>>> found in the context. So entry is null.
>> 
>> That's obviously a bug in the 2LC testsuite, isn't it?
> 
> LOL, is it? You added the class and the javadoc clearly states that this 
> entity defines equals incorrectly. You must have added it for a reason?
> 
> https://github.com/hibernate/hibernate-orm/blob/master/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/entities/Name.java
> 
> In any case, an object with an incorrect equals impl should not result in an 
> NPE within Infinispan :|
> 
>> Object used as @EmbeddedId needs to have the equals correctly defined. How 
>> else would you compare ids? I wonder how could that work in the past.
> 
> ROFL... it's your baby so you tell me ;)
> 
>> 
>>> 
>>> Moreover, if EntryLookup.lookupEntry javadoc (and some implementations) can 
>>> and do return null. Are you expecting that somehow that method will never 
>>> return null?
>> 
>> With ISPN-7029 in, the entry should be wrapped in the context after 
>> EntryWrappingInterceptor if the key is in readCH, otherwise it should be 
>> null. In case that xDistributionInterceptor finds out that it needs to work 
>> on that value despite not being able to read it (e.g. in case that it's in 
>> writeCH during unfinished rebalance), it should wrap 
>> NullCacheEntry.getInstance() using EntryFactory. wrapExternalEntry. More 
>> info about the logic is in o.i.c.EntryFactory javadoc [3].
> 
> Not sure I understand what you're trying to imply above... so, is 
> lookupEntry() allowed to return null or not?
> 
> To be more specific, SingleKeyNonTxInvocationContext.lookupEntry() can return 
> null, so GetKeyValueCommand should be able to handle it? Or should 
> SingleKeyNonTxInvocationContext.lookupEntry return NullCacheEntry.getInstance 
> instead of null? 
> 
> To provide more specifics, SingleKeyNonTxInvocationContext has 
> NullCacheEntry.getInstance in cacheEntry variable when it's returning null. 
> Should it maybe return NullCacheEntry.getInstance instead of null from 
> lookupEntry() ?
> 
> Cheers,
> 
>> 
>> Radim
>> 
>> [3] 
>> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/container/EntryFactory.java
>> 
>>> 
>>> I'll create a JIRA to track all issues arising from Hibernate 2LC in a 
>>> minute, but wanted to get your thoughts firts.
>>> 
>>> Cheers,
>>> 
>>> [1] https://issues.jboss.org/browse/ISPN-7029
>>> [2] 
>>> https://github.com/hibernate/hibernate-orm/blob/master/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/CacheKeysFactoryTest.java#L58
>>> --
>>> Galder Zamarreño
>>> Infinispan, Red Hat
>>> 
>> 
>> 
>> -- 
>> Radim Vansa <rva...@redhat.com>
>> JBoss Performance Team
> 
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

Reply via email to