ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Pavlukhina <vololo...@gmail.com>
Subject Re: CacheInterceptor ClassCastException in case of cache was updated from thin java client
Date Thu, 24 Jan 2019 19:25:54 GMT
Sergey,

There are couple of things which should be addressed:
1. Unnecessary deserialization.
2. Inconsistent behavior.
3. Unclear documentation.

Deserialization is not free and in my mind should be avoided where possible. I think that
if some feature (like interceptors) requires deserialization then it should be enabled explicitly
and an impact should be clear to user. I can imagine a toggle “withAllowedDeserialization”.

If there is an inconsistency between thick and thin clients it should be eliminated. I do
not see a reason why behavior should be different.

If something is a good thing but it is not intuitive it could be documented. But if there
is s really good reason for it. Otherwise simplicity and consistency are better alias.

> On 24 Jan 2019, at 17:42, Sergey Antonov <antonovsergey93@gmail.com> wrote:
> 
> I think it's bad idea. This contract nowhere defined and it's not clear for
> users.
> 
> чт, 24 янв. 2019 г. в 17:18, Pavel Tupitsyn <ptupitsyn@apache.org>:
> 
>> Yes
>> 
>> On Thu, Jan 24, 2019 at 5:15 PM Sergey Antonov <antonovsergey93@gmail.com>
>> wrote:
>> 
>>> Pavel,
>>> 
>>> "Leave it as is, use instanceof."
>>> You meant always use CacheInterceptor<Object, Object> and in all methods
>>> check, that passed arguments is BinaryObject?
>>> 
>>> чт, 24 янв. 2019 г. в 17:10, Pavel Tupitsyn <ptupitsyn@apache.org>:
>>> 
>>>> I don't think we should complicate things. Leave it as is, use
>>> instanceof.
>>>> The fact is - you can get anything, BinaryObject or any user class, so
>> be
>>>> prepared.
>>>> Good example of older API is CacheEvent, which actually has oldValue()
>>> and
>>>> newValue() as Object.
>>>> 
>>>> Igniters, any other thoughts?
>>>> 
>>>> 
>>>> On Thu, Jan 24, 2019 at 2:16 PM Sergey Antonov <
>>> antonovsergey93@gmail.com>
>>>> wrote:
>>>> 
>>>>> Pavel, how about marker interface DeserializedValueCacheInterceptor?
>> We
>>>>> will deserialize data and pass it to cache interceptor, if
>>>> CacheInterceptor
>>>>> implements marker interface.
>>>>> 
>>>>> чт, 24 янв. 2019 г. в 13:41, Pavel Tupitsyn <ptupitsyn@apache.org>:
>>>>> 
>>>>>> You are exactly right, generic parameters don't make much sense
>> here.
>>>>>> Ignite caches are not restricted to any type, and there is type
>>> erasure
>>>>> in
>>>>>> Java so you have no runtime guarantees.
>>>>>> 
>>>>>> Maybe Interceptor design should be improved (e.g. add a flag to
>> force
>>>>>> binary or non-binary mode),
>>>>>> but Thin or Thick client connector logic is unrelated here.
>>>>>> withKeepBinary() call is valid and should not depend on Interceptor
>>>>>> presence or implementation.
>>>>>> 
>>>>>> On Thu, Jan 24, 2019 at 1:17 PM Sergey Antonov <
>>>>> antonovsergey93@gmail.com>
>>>>>> wrote:
>>>>>> 
>>>>>>> Hi, Pavel,
>>>>>>> 
>>>>>>> "Interceptor should support both modes, binary or not. Any code
>> can
>>>>> call
>>>>>>> withKeepBinary(), this should be expected.
>>>>>>> Just add if (x instanceof BinaryObject) and go from there. "
>>>>>>> I don't agree. The cache interceptor[1] is a parametrized class
>> and
>>>> you
>>>>>>> couldn't pass multiple cache interceptors in cache configuration.
>>> So
>>>>> all
>>>>>>> cache interceptors must have Object, Object parameters for
>>> supporting
>>>>>> both
>>>>>>> modes: binary and deserialized. In this case parametrized class
>> no
>>>>> sense.
>>>>>>> 
>>>>>>> [1]
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/CacheInterceptor.html
>>>>>>> 
>>>>>>> чт, 24 янв. 2019 г. в 13:06, Pavel Tupitsyn <
>> ptupitsyn@apache.org
>>>> :
>>>>>>> 
>>>>>>>> Hi Sergey,
>>>>>>>> 
>>>>>>>> I don't think this is a bug.
>>>>>>>> 
>>>>>>>> Thick or thin clients always work in binary mode on server
>> side,
>>>>>> because
>>>>>>>> you receive data in serialized form and there is no point
in
>>>>>>> deserializing
>>>>>>>> it.
>>>>>>>> Moreover, in most cases you don't have classes on the server,
>> so
>>>>> binary
>>>>>>>> mode is the only way.
>>>>>>>> 
>>>>>>>> Interceptor should support both modes, binary or not. Any
code
>>> can
>>>>> call
>>>>>>>> withKeepBinary(), this should be expected.
>>>>>>>> Just add if (x instanceof BinaryObject) and go from there.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Pavel
>>>>>>>> 
>>>>>>>> On Thu, Jan 24, 2019 at 12:38 PM Sergey Antonov <
>>>>>>> antonovsergey93@gmail.com
>>>>>>>>> 
>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>>> I did a little investigation. In
>>>>>>>> o.a.i.i.p.p.c.c.ClientCacheRequest#cache()
>>>>>>>>> enforced cache with keep binary. Why we should always
work
>>> binary
>>>>>>>> objects?
>>>>>>>>> 
>>>>>>>>> чт, 24 янв. 2019 г. в 12:29, Sergey Antonov <
>>>>>> antonovsergey93@gmail.com
>>>>>>>> :
>>>>>>>>> 
>>>>>>>>>> Hello, Igniters!
>>>>>>>>>> 
>>>>>>>>>> I have ignite node with configured cache. The cache
have
>>> cache
>>>>>>>>>> interceptor. I wiil got ClassCastException on cache
>>>> interceptor,
>>>>>> If I
>>>>>>>> put
>>>>>>>>>> some entry to the cache (without keepBinary) from
thin java
>>>>> client.
>>>>>>>>>> 
>>>>>>>>>> I think it's a bug. I'd like to find out yours view!
>>>>>>>>>> 
>>>>>>>>>> Also I made JIRA ticket with reproducer [1].
>>>>>>>>>> 
>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-10789
>>>>>>>>>> 
>>>>>>>>>> --
>>>>>>>>>> BR, Sergey Antonov
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> --
>>>>>>>>> BR, Sergey Antonov
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> BR, Sergey Antonov
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> BR, Sergey Antonov
>>>>> 
>>>> 
>>> 
>>> 
>>> --
>>> BR, Sergey Antonov
>>> 
>> 
> 
> 
> -- 
> BR, Sergey Antonov

Mime
View raw message