Storing deserialized value when indexing is enabled

[Valentin Kulichenko]

Folks,

It looks like we cache the deserialized value in case indexing is enabled
and a read operation happens on server side (e.g. affinity closure does
get() on primary node).

This must be caused by this piece of code in IgniteCacheObjectProcessorImpl:

boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
    GridQueryProcessor.isEnabled(ccfg) ||
    !ccfg.isCopyOnRead();

This actually means that we store deserialized values in case indexing is
configured even if copyOnRead is true. My understanding is that this is a
bug and GridQueryProcessor.isEnabled(ccfg) condition should be removed from
there (at least for the case when binary marshaller is used).

Am I missing something?

-Val

[dmagda]

Val,

You get confused by IgniteCacheObjectProcessorImpl.contextForCache(…) method.

When binary marshaller is enable the default processor is CacheObjectBinaryProcessorImpl and when contextForCache(…) gets called on its side the processor will create CacheObjectBinaryContext which, in its turn, has binaryEnabled field that in fact controls an object deserialization on a server side.

—
Denis
 

> On Nov 23, 2016, at 12:14 PM, Valentin Kulichenko <[hidden email]> wrote:
>
> Folks,
>
> It looks like we cache the deserialized value in case indexing is enabled
> and a read operation happens on server side (e.g. affinity closure does
> get() on primary node).
>
> This must be caused by this piece of code in IgniteCacheObjectProcessorImpl:
>
> boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
>    GridQueryProcessor.isEnabled(ccfg) ||
>    !ccfg.isCopyOnRead();
>
> This actually means that we store deserialized values in case indexing is
> configured even if copyOnRead is true. My understanding is that this is a
> bug and GridQueryProcessor.isEnabled(ccfg) condition should be removed from
> there (at least for the case when binary marshaller is used).
>
> Am I missing something?
>
> -Val

[Valentin Kulichenko]

Denis,

CacheObjectBinaryProcessorImpl calls super and preserves the value of
storeVal flag. It seems to me that it's then used in
BinaryObjectImpl.deserializeValue() causing the deserialized value to be
saved in memory. This doubles memory consumption in this scenario.

I created a ticket: https://issues.apache.org/jira/browse/IGNITE-4293

-Val

On Wed, Nov 23, 2016 at 12:32 PM, Denis Magda <[hidden email]> wrote:

> Val,
>
> You get confused by IgniteCacheObjectProcessorImpl.contextForCache(…)
> method.
>
> When binary marshaller is enable the default processor is
> CacheObjectBinaryProcessorImpl and when contextForCache(…) gets called on
> its side the processor will create CacheObjectBinaryContext which, in its
> turn, has binaryEnabled field that in fact controls an object
> deserialization on a server side.
>
> —
> Denis
>
> > On Nov 23, 2016, at 12:14 PM, Valentin Kulichenko <
> [hidden email]> wrote:
> >
> > Folks,
> >
> > It looks like we cache the deserialized value in case indexing is enabled
> > and a read operation happens on server side (e.g. affinity closure does
> > get() on primary node).
> >
> > This must be caused by this piece of code in
> IgniteCacheObjectProcessorImpl:
> >
> > boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
> >    GridQueryProcessor.isEnabled(ccfg) ||
> >    !ccfg.isCopyOnRead();
> >
> > This actually means that we store deserialized values in case indexing is
> > configured even if copyOnRead is true. My understanding is that this is a
> > bug and GridQueryProcessor.isEnabled(ccfg) condition should be removed
> from
> > there (at least for the case when binary marshaller is used).
> >
> > Am I missing something?
> >
> > -Val
>
>

[dmagda]

Val,

Yeah, the code is like jungles in some places. Proposed a possible fix inside of the ticket.

—
Denis

> On Nov 23, 2016, at 1:01 PM, Valentin Kulichenko <[hidden email]> wrote:
>
> Denis,
>
> CacheObjectBinaryProcessorImpl calls super and preserves the value of
> storeVal flag. It seems to me that it's then used in
> BinaryObjectImpl.deserializeValue() causing the deserialized value to be
> saved in memory. This doubles memory consumption in this scenario.
>
> I created a ticket: https://issues.apache.org/jira/browse/IGNITE-4293
>
> -Val
>
> On Wed, Nov 23, 2016 at 12:32 PM, Denis Magda <[hidden email]> wrote:
>
>> Val,
>>
>> You get confused by IgniteCacheObjectProcessorImpl.contextForCache(…)
>> method.
>>
>> When binary marshaller is enable the default processor is
>> CacheObjectBinaryProcessorImpl and when contextForCache(…) gets called on
>> its side the processor will create CacheObjectBinaryContext which, in its
>> turn, has binaryEnabled field that in fact controls an object
>> deserialization on a server side.
>>
>> —
>> Denis
>>
>>> On Nov 23, 2016, at 12:14 PM, Valentin Kulichenko <
>> [hidden email]> wrote:
>>>
>>> Folks,
>>>
>>> It looks like we cache the deserialized value in case indexing is enabled
>>> and a read operation happens on server side (e.g. affinity closure does
>>> get() on primary node).
>>>
>>> This must be caused by this piece of code in
>> IgniteCacheObjectProcessorImpl:
>>>
>>> boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
>>>   GridQueryProcessor.isEnabled(ccfg) ||
>>>   !ccfg.isCopyOnRead();
>>>
>>> This actually means that we store deserialized values in case indexing is
>>> configured even if copyOnRead is true. My understanding is that this is a
>>> bug and GridQueryProcessor.isEnabled(ccfg) condition should be removed
>> from
>>> there (at least for the case when binary marshaller is used).
>>>
>>> Am I missing something?
>>>
>>> -Val
>>
>>

[Pavel Tupitsyn]

Looks like a known issue to me, I've linked related tickets in Jira:

https://issues.apache.org/jira/browse/IGNITE-2417
https://issues.apache.org/jira/browse/IGNITE-3347

On Thu, Nov 24, 2016 at 12:13 AM, Denis Magda <[hidden email]> wrote:

> Val,
>
> Yeah, the code is like jungles in some places. Proposed a possible fix
> inside of the ticket.
>
> —
> Denis
>
> > On Nov 23, 2016, at 1:01 PM, Valentin Kulichenko <
> [hidden email]> wrote:
> >
> > Denis,
> >
> > CacheObjectBinaryProcessorImpl calls super and preserves the value of
> > storeVal flag. It seems to me that it's then used in
> > BinaryObjectImpl.deserializeValue() causing the deserialized value to be
> > saved in memory. This doubles memory consumption in this scenario.
> >
> > I created a ticket: https://issues.apache.org/jira/browse/IGNITE-4293
> >
> > -Val
> >
> > On Wed, Nov 23, 2016 at 12:32 PM, Denis Magda <[hidden email]> wrote:
> >
> >> Val,
> >>
> >> You get confused by IgniteCacheObjectProcessorImpl.contextForCache(…)
> >> method.
> >>
> >> When binary marshaller is enable the default processor is
> >> CacheObjectBinaryProcessorImpl and when contextForCache(…) gets called
> on
> >> its side the processor will create CacheObjectBinaryContext which, in
> its
> >> turn, has binaryEnabled field that in fact controls an object
> >> deserialization on a server side.
> >>
> >> —
> >> Denis
> >>
> >>> On Nov 23, 2016, at 12:14 PM, Valentin Kulichenko <
> >> [hidden email]> wrote:
> >>>
> >>> Folks,
> >>>
> >>> It looks like we cache the deserialized value in case indexing is
> enabled
> >>> and a read operation happens on server side (e.g. affinity closure does
> >>> get() on primary node).
> >>>
> >>> This must be caused by this piece of code in
> >> IgniteCacheObjectProcessorImpl:
> >>>
> >>> boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
> >>>   GridQueryProcessor.isEnabled(ccfg) ||
> >>>   !ccfg.isCopyOnRead();
> >>>
> >>> This actually means that we store deserialized values in case indexing
> is
> >>> configured even if copyOnRead is true. My understanding is that this
> is a
> >>> bug and GridQueryProcessor.isEnabled(ccfg) condition should be removed
> >> from
> >>> there (at least for the case when binary marshaller is used).
> >>>
> >>> Am I missing something?
> >>>
> >>> -Val
> >>
> >>
>
>