keepBinaryInStore

[Alexey Goncharuk]

Igniters,

We introduced a cache configuration property keepBinaryInStore which was
meant to determine the way persisted objects are passed to the store. This
flag overrides the value of withKeepBinary() projection flag, i.e. the
current behavior is:

withKeepBinary=false, keepBinaryInStore=false, store receives Deserialized
values
withKeepBinary=true, keepBinaryInStore=false, store receives Deserialized
values
withKeepBinary=false, keepBinaryInStore=true, store receives BinaryObjects
withKeepBinary=true, keepBinaryInStore=true, store receives BinaryObjects

However, from the -b1 user feedback it looks like this configuration
property and behavior is confusing. I think it makes sense to remove the
configuration property and rely solely on the projection flag, i.e.

withKeepBinary=false, store receives Deserialized values
withKeepBinary=true, store receives BinaryObjects

which is much simpler to understand.

Am I missing something?

--AG

[Vladimir Ozerov]

Alex,

What should happen if user do this?

Cache c1 = ...;
c1.put(...);
Cache c2 = c1.withKeepBinary();
c2.put(...);
10 дек. 2015 г. 17:02 пользователь "Alexey Goncharuk" <
[hidden email]> написал:

> Igniters,
>
> We introduced a cache configuration property keepBinaryInStore which was
> meant to determine the way persisted objects are passed to the store. This
> flag overrides the value of withKeepBinary() projection flag, i.e. the
> current behavior is:
>
> withKeepBinary=false, keepBinaryInStore=false, store receives Deserialized
> values
> withKeepBinary=true, keepBinaryInStore=false, store receives Deserialized
> values
> withKeepBinary=false, keepBinaryInStore=true, store receives BinaryObjects
> withKeepBinary=true, keepBinaryInStore=true, store receives BinaryObjects
>
> However, from the -b1 user feedback it looks like this configuration
> property and behavior is confusing. I think it makes sense to remove the
> configuration property and rely solely on the projection flag, i.e.
>
> withKeepBinary=false, store receives Deserialized values
> withKeepBinary=true, store receives BinaryObjects
>
> which is much simpler to understand.
>
> Am I missing something?
>
> --AG
>

[Alexey Goncharuk]

>
> Alex,
>
> What should happen if user do this?
>
> Cache c1 = ...;
> c1.put(...);
>
Store will receive Deserialized value


> Cache c2 = c1.withKeepBinary();
> c2.put(...);
>

Store will receive BinaryObject

Of course in this case the store must have <Object, Object> generic types,
but I see no issues with this.

[Alexey Kuznetsov-2]

Alex,

How user should put objects in cache in this case:

1) One node started with user classes in classpath.
2) Second node started WITHOUT user classes in classpath.
3) Cache configured with store.

On first node user put objects in cache.

Should user do puts via c1.withKeepBinary() ? in order to not get
ClassNotFoundException?


--
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

[Artem Shutak]

Huge +1 for Alexey's offer. When you did not know about this property, you
will get an obscure exception on put operation [1] for the following code:

        IgniteCache<Integer, BinaryObject> cache =
ignite.cache(null).withKeepBinary();
        BinaryObject o =
ignite.binary().builder("orders").setField("fname", "value").build();
        cache.put(1, o);

To fix it you need to set CacheConfiguration.keepBinaryInStore(true). It
looks like a usability issue, because I already said 'withKeepPortable' and
Ignite should use this information.

For example, old Portables in GridGain doesn't have this requirement.

[1]
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxLocalAdapter.batchStoreCommit(IgniteTxLocalAdapter.java:822)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxLocalAdapter.userCommit(IgniteTxLocalAdapter.java:859)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal.finish(GridNearTxLocal.java:738)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxFinishFuture.finish(GridNearTxFinishFuture.java:331)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$4.apply(GridNearTxLocal.java:838)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxLocal$4.apply(GridNearTxLocal.java:830)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListener(GridFutureAdapter.java:262)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListeners(GridFutureAdapter.java:250)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:380)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:346)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture.onComplete(GridNearOptimisticTxPrepareFuture.java:225)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture.onDone(GridNearOptimisticTxPrepareFuture.java:203)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture.onDone(GridNearOptimisticTxPrepareFuture.java:66)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:323)
at
org.apache.ignite.internal.util.future.GridCompoundFuture.checkComplete(GridCompoundFuture.java:301)
at
org.apache.ignite.internal.util.future.GridCompoundFuture.access$400(GridCompoundFuture.java:41)
at
org.apache.ignite.internal.util.future.GridCompoundFuture$Listener.apply(GridCompoundFuture.java:421)
at
org.apache.ignite.internal.util.future.GridCompoundFuture$Listener.apply(GridCompoundFuture.java:365)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListener(GridFutureAdapter.java:262)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListeners(GridFutureAdapter.java:250)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:380)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:346)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:323)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture$MiniFuture.onResult(GridNearOptimisticTxPrepareFuture.java:741)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture$1.apply(GridNearOptimisticTxPrepareFuture.java:466)
at
org.apache.ignite.internal.processors.cache.distributed.near.GridNearOptimisticTxPrepareFuture$1.apply(GridNearOptimisticTxPrepareFuture.java:463)
at
org.apache.ignite.internal.util.future.GridFutureAdapter$ArrayListener.apply(GridFutureAdapter.java:440)
at
org.apache.ignite.internal.util.future.GridFutureAdapter$ArrayListener.apply(GridFutureAdapter.java:423)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListener(GridFutureAdapter.java:262)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListeners(GridFutureAdapter.java:250)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:380)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:346)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture.onComplete(GridDhtTxPrepareFuture.java:855)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture.onDone(GridDhtTxPrepareFuture.java:695)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture.onDone(GridDhtTxPrepareFuture.java:95)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:323)
at
org.apache.ignite.internal.util.future.GridCompoundFuture.checkComplete(GridCompoundFuture.java:301)
at
org.apache.ignite.internal.util.future.GridCompoundFuture.access$400(GridCompoundFuture.java:41)
at
org.apache.ignite.internal.util.future.GridCompoundFuture$Listener.apply(GridCompoundFuture.java:421)
at
org.apache.ignite.internal.util.future.GridCompoundFuture$Listener.apply(GridCompoundFuture.java:365)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListener(GridFutureAdapter.java:262)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.notifyListeners(GridFutureAdapter.java:250)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:380)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:346)
at
org.apache.ignite.internal.util.future.GridFutureAdapter.onDone(GridFutureAdapter.java:323)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture$MiniFuture.onResult(GridDhtTxPrepareFuture.java:1587)
at
org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture.onResult(GridDhtTxPrepareFuture.java:469)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.processDhtTxPrepareResponse(IgniteTxHandler.java:524)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.access$200(IgniteTxHandler.java:88)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler$6.apply(IgniteTxHandler.java:145)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler$6.apply(IgniteTxHandler.java:143)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.processMessage(GridCacheIoManager.java:579)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.onMessage0(GridCacheIoManager.java:277)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.handleMessage(GridCacheIoManager.java:201)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager.access$000(GridCacheIoManager.java:80)
at
org.apache.ignite.internal.processors.cache.GridCacheIoManager$1.onMessage(GridCacheIoManager.java:163)
at
org.apache.ignite.internal.managers.communication.GridIoManager.processRegularMessage0(GridIoManager.java:819)
at
org.apache.ignite.internal.managers.communication.GridIoManager.access$1600(GridIoManager.java:103)
at
org.apache.ignite.internal.managers.communication.GridIoManager$5.run(GridIoManager.java:782)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
... 1 more
Caused by: class org.apache.ignite.binary.BinaryObjectException: Failed
resolve class for ID: -1008770331
at
org.apache.ignite.internal.portable.PortableContext.descriptorForTypeId(PortableContext.java:484)
at
org.apache.ignite.internal.portable.BinaryReaderExImpl.deserialize(BinaryReaderExImpl.java:1429)
at
org.apache.ignite.internal.portable.BinaryObjectImpl.deserializeValue(BinaryObjectImpl.java:551)
at
org.apache.ignite.internal.portable.BinaryObjectImpl.value(BinaryObjectImpl.java:131)
at
org.apache.ignite.internal.processors.cache.CacheObjectContext.unwrapPortable(CacheObjectContext.java:280)
at
org.apache.ignite.internal.processors.cache.CacheObjectContext.unwrapPortableIfNeeded(CacheObjectContext.java:145)
at
org.apache.ignite.internal.processors.cache.CacheObjectContext.unwrapPortableIfNeeded(CacheObjectContext.java:132)
at
org.apache.ignite.internal.processors.cache.GridCacheContext.unwrapPortableIfNeeded(GridCacheContext.java:1748)
at
org.apache.ignite.internal.processors.cache.store.GridCacheStoreManagerAdapter.put(GridCacheStoreManagerAdapter.java:537)
at
org.apache.ignite.internal.processors.cache.store.GridCacheStoreManagerAdapter.putAll(GridCacheStoreManagerAdapter.java:582)
at
org.apache.ignite.internal.processors.cache.transactions.IgniteTxLocalAdapter.batchStoreCommit(IgniteTxLocalAdapter.java:786)
... 61 more
Caused by: class org.apache.ignite.IgniteCheckedException: Failed to read
class name from file [id=-1008770331,
file=/tmp/ignite/work/marshaller/-1008770331.classname]
at
org.apache.ignite.internal.MarshallerContextImpl.className(MarshallerContextImpl.java:158)
at
org.apache.ignite.internal.MarshallerContextAdapter.getClass(MarshallerContextAdapter.java:174)
at
org.apache.ignite.internal.portable.PortableContext.descriptorForTypeId(PortableContext.java:468)
... 71 more
Caused by: java.io.FileNotFoundException:
/tmp/ignite/work/marshaller/-1008770331.classname (No such file or
directory)
at java.io.FileInputStream.open(Native Method)
at java.io.FileInputStream.<init>(FileInputStream.java:146)
at java.io.FileReader.<init>(FileReader.java:72)
at
org.apache.ignite.internal.MarshallerContextImpl.className(MarshallerContextImpl.java:154)
... 73 more

Thanks,
-- Artem --

On Thu, Dec 10, 2015 at 5:14 PM, Alexey Goncharuk <
[hidden email]> wrote:

> >
> > Alex,
> >
> > What should happen if user do this?
> >
> > Cache c1 = ...;
> > c1.put(...);
> >
> Store will receive Deserialized value
>
>
> > Cache c2 = c1.withKeepBinary();
> > c2.put(...);
> >
>
> Store will receive BinaryObject
>
> Of course in this case the store must have <Object, Object> generic types,
> but I see no issues with this.
>

[Alexey Goncharuk]

Alexey,

Yes, I think in this case user should do puts on a cache obtained by a call
to withKeepBinary(). This behavior is not related to cache store only -
withKeepBinary is already honored this way for EntryProcessor,
CacheInterceptor, etc. I do not see why CacheStore should be an exception.

[Denis Magda]

Guys,

Take a look at this situation from a different view.

The following code snippet provided by Vladimir will guarantee that in
both cases "put" calls will store data in the binary format in a cache
(if BinaryMarshaller is used by default).

Cache c1 = ...;
c1.put(...);
Cache c2 = c1.withKeepBinary();
c2.put(...);

This guarantee sounds natural for me because I'm as a user don't want to
run into the situation when an entry is stored not in the binary format
just because I forgot to use "withKeepBinary()" at some point.

In my understanding the same approach should be implemented for the
storage as well. If "keepPortableInStore" is set to "true" then entries
are always stored in the binary format regardless on whether I used
"withKeepBinary()" or not.
I doubt that the user may want to keep data in the storage in both the
binary and non binary formats. So I don't see any reason why
"withKeepBinary()" is useful for the storage at all (the same as for
cache.puts).

--
Denis

On 12/10/2015 5:05 PM, Vladimir Ozerov wrote:

> Alex,
>
> What should happen if user do this?
>
> Cache c1 = ...;
> c1.put(...);
> Cache c2 = c1.withKeepBinary();
> c2.put(...);
> 10 дек. 2015 г. 17:02 пользователь "Alexey Goncharuk" <
> [hidden email]> написал:
>
>> Igniters,
>>
>> We introduced a cache configuration property keepBinaryInStore which was
>> meant to determine the way persisted objects are passed to the store. This
>> flag overrides the value of withKeepBinary() projection flag, i.e. the
>> current behavior is:
>>
>> withKeepBinary=false, keepBinaryInStore=false, store receives Deserialized
>> values
>> withKeepBinary=true, keepBinaryInStore=false, store receives Deserialized
>> values
>> withKeepBinary=false, keepBinaryInStore=true, store receives BinaryObjects
>> withKeepBinary=true, keepBinaryInStore=true, store receives BinaryObjects
>>
>> However, from the -b1 user feedback it looks like this configuration
>> property and behavior is confusing. I think it makes sense to remove the
>> configuration property and rely solely on the projection flag, i.e.
>>
>> withKeepBinary=false, store receives Deserialized values
>> withKeepBinary=true, store receives BinaryObjects
>>
>> which is much simpler to understand.
>>
>> Am I missing something?
>>
>> --AG
>>

[dsetrakyan]

I actually think we should take a bit different approach here.

How about adding CacheBinaryStore interface which will always receive
binary objects? In this case, user has an option of either implementing
CacheStore or CacheBinaryStore (which extends CacheStore). Based on the
implementation, we can decide on whether we pass objects in serialized
binary form or in deserialized Java form.

BTW, I don’t think we should take *withKeepBinary()* flag into account at
all when working with a CacheStore. As Vladimir pointed out, the same API
can be called with or without the binary format enabled, so basing
CacheStore logic on how user decides to call the API seems ill-suited and
hacky to me.

Thoughts?

D.

On Thu, Dec 10, 2015 at 11:52 AM, Denis Magda <[hidden email]> wrote:

> Guys,
>
> Take a look at this situation from a different view.
>
> The following code snippet provided by Vladimir will guarantee that in
> both cases "put" calls will store data in the binary format in a cache (if
> BinaryMarshaller is used by default).
>
> Cache c1 = ...;
> c1.put(...);
> Cache c2 = c1.withKeepBinary();
> c2.put(...);
>
> This guarantee sounds natural for me because I'm as a user don't want to
> run into the situation when an entry is stored not in the binary format
> just because I forgot to use "withKeepBinary()" at some point.
>
> In my understanding the same approach should be implemented for the
> storage as well. If "keepPortableInStore" is set to "true" then entries are
> always stored in the binary format regardless on whether I used
> "withKeepBinary()" or not.
> I doubt that the user may want to keep data in the storage in both the
> binary and non binary formats. So I don't see any reason why
> "withKeepBinary()" is useful for the storage at all (the same as for
> cache.puts).
>
> --
> Denis
>
>
> On 12/10/2015 5:05 PM, Vladimir Ozerov wrote:
>
>> Alex,
>>
>> What should happen if user do this?
>>
>> Cache c1 = ...;
>> c1.put(...);
>> Cache c2 = c1.withKeepBinary();
>> c2.put(...);
>> 10 дек. 2015 г. 17:02 пользователь "Alexey Goncharuk" <
>> [hidden email]> написал:
>>
>> Igniters,
>>>
>>> We introduced a cache configuration property keepBinaryInStore which was
>>> meant to determine the way persisted objects are passed to the store.
>>> This
>>> flag overrides the value of withKeepBinary() projection flag, i.e. the
>>> current behavior is:
>>>
>>> withKeepBinary=false, keepBinaryInStore=false, store receives
>>> Deserialized
>>> values
>>> withKeepBinary=true, keepBinaryInStore=false, store receives Deserialized
>>> values
>>> withKeepBinary=false, keepBinaryInStore=true, store receives
>>> BinaryObjects
>>> withKeepBinary=true, keepBinaryInStore=true, store receives BinaryObjects
>>>
>>> However, from the -b1 user feedback it looks like this configuration
>>> property and behavior is confusing. I think it makes sense to remove the
>>> configuration property and rely solely on the projection flag, i.e.
>>>
>>> withKeepBinary=false, store receives Deserialized values
>>> withKeepBinary=true, store receives BinaryObjects
>>>
>>> which is much simpler to understand.
>>>
>>> Am I missing something?
>>>
>>> --AG
>>>
>>>
>

[Alexey Kuznetsov-2]

How about always pass value in binary format to store in case of
BinaryMarshaller?
Why do we need deserialized object in store?

--
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

[dsetrakyan]

On Thu, Dec 10, 2015 at 6:26 PM, Alexey Kuznetsov <[hidden email]>
wrote:

> How about always pass value in binary format to store in case of
> BinaryMarshaller? Why do we need deserialized object in store?


This would not be backward compatible. Essentially, everyone switching to
the new version of Ignite would be required to rewrite their store
implementation. I still think that introducing CacheBinaryStore is the most
elegant solution here.

D.

[Alexey Goncharuk]

>
>
> This would not be backward compatible. Essentially, everyone switching to
> the new version of Ignite would be required to rewrite their store
> implementation. I still think that introducing CacheBinaryStore is the most
> elegant solution here.


Dmitriy,

Do I understand correctly that you suggest CacheBinaryStore be just an
empty extension of CacheStore interface?

[dsetrakyan]

On Thu, Dec 10, 2015 at 10:24 PM, Alexey Goncharuk <
[hidden email]> wrote:

> >
> >
> > This would not be backward compatible. Essentially, everyone switching to
> > the new version of Ignite would be required to rewrite their store
> > implementation. I still think that introducing CacheBinaryStore is the
> most
> > elegant solution here.
>
>
> Dmitriy,
>
> Do I understand correctly that you suggest CacheBinaryStore be just an
> empty extension of CacheStore interface?
>

Yes, with proper documentation explaining its purpose.

[Alexey Goncharuk]

If we do not want withKeepBinary() flag to affect the way objects are
passed to a cache store, then I think current approach with a cache
configuration flag is better than having a separate interface for store.

With separate interfaces it is impossible to implement a 'universal' store
which will properly handle both deserialized objects and binary objects,
like our JDBC pojo store. Moreover, it is not clear to me what should
happen if BinaryObjectStore is plugged and OptimizedMarshaller is set. This
will force user to switch cache store implementation when marshaller
changes, which does not sound right to me.

[dsetrakyan]

On Thu, Dec 10, 2015 at 11:44 PM, Alexey Goncharuk <
[hidden email]> wrote:

> If we do not want withKeepBinary() flag to affect the way objects are
> passed to a cache store, then I think current approach with a cache
> configuration flag is better than having a separate interface for store.
>
> With separate interfaces it is impossible to implement a 'universal' store
> which will properly handle both deserialized objects and binary objects,
> like our JDBC pojo store. Moreover, it is not clear to me what should
> happen if BinaryObjectStore is plugged and OptimizedMarshaller is set. This
> will force user to switch cache store implementation when marshaller
> changes, which does not sound right to me.
>

Agree, good catch! In this case we should rename it to be consistent with
out store configuration properties, e.g. setStoreKeepBinary(…)