You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Dmitriy Govorukhin <dm...@gmail.com> on 2017/04/11 10:06:17 UTC
CachePojoStore data loading with binary marshaller
Hi all,
I found problem related to CacheJdbcPojoStore. If try to load data, with
enabled binary marshaller,
error may occur. Problem in entryMappings (this map helps resolve stored
type when data loading).
If binary marshaller disable we just put entry with java type in this map,
if enable we put java type + binary type.
......
checkTypeConfiguration(cacheName, valKind, valType,
type.getValueFields());
entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect, type,
keyKind, valKind, sqlEscapeAll));
// Add one more binding to binary typeId for POJOs,
// because object could be passed to store in binary format.
if (binarySupported && keyKind == TypeKind.POJO) {
keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
type, TypeKind.BINARY, valKind, sqlEscapeAll));
}
.......
I think it is wrong, because after we use this map like iterator for load,
and final result dependent from which mapping will be first java or binary.
Collection<String> processedKeyTypes = new HashSet<>();
for (EntryMapping em : mappings.values()) {
String keyType = em.keyType();
if (processedKeyTypes.contains(keyType))
continue;
processedKeyTypes.add(keyType);
.....
I think we must add only binary mapping for user pojo if binary marshaller
enable.
Re: CachePojoStore data loading with binary marshaller
Posted by Valentin Kulichenko <va...@gmail.com>.
Yes, that's my thinking as well.
-Val
On Wed, Apr 12, 2017 at 10:59 AM, Dmitriy Govorukhin <
dmitriy.govorukhin@gmail.com> wrote:
> Hi Valentin,
>
> Other word, are you agree with me? My point that we always must use binary
> representation if binary marshaller enable.
>
> On Wed, Apr 12, 2017 at 10:24 AM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
>
> > Hi Dmitry,
> >
> > I can't advise a lot on implementation details, but generally I don't
> think
> > that CacheJdbcPojoStore should ever work with POJO classes in case binary
> > marshaller is enabled. It should always work directly with binary objects
> > even if POJOs are on server classpath, and switch to POJOs only when
> other
> > marshaller is used.
> >
> > Makes sense?
> >
> > -Val
> >
> > On Tue, Apr 11, 2017 at 12:06 PM, Dmitriy Govorukhin <
> > dmitriy.govorukhin@gmail.com> wrote:
> >
> > > Hi all,
> > >
> > > I found problem related to CacheJdbcPojoStore. If try to load data,
> with
> > > enabled binary marshaller,
> > > error may occur. Problem in entryMappings (this map helps resolve
> stored
> > > type when data loading).
> > > If binary marshaller disable we just put entry with java type in this
> > map,
> > > if enable we put java type + binary type.
> > >
> > > ......
> > >
> > > checkTypeConfiguration(cacheName, valKind, valType,
> > > type.getValueFields());
> > >
> > > entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> > type,
> > > keyKind, valKind, sqlEscapeAll));
> > >
> > > // Add one more binding to binary typeId for POJOs,
> > > // because object could be passed to store in binary format.
> > > if (binarySupported && keyKind == TypeKind.POJO) {
> > > keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
> > >
> > > valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
> > >
> > > entryMappings.put(keyTypeId, new EntryMapping(cacheName,
> dialect,
> > > type, TypeKind.BINARY, valKind, sqlEscapeAll));
> > > }
> > >
> > > .......
> > >
> > >
> > > I think it is wrong, because after we use this map like iterator for
> > load,
> > > and final result dependent from which mapping will be first java or
> > binary.
> > >
> > > Collection<String> processedKeyTypes = new HashSet<>();
> > >
> > > for (EntryMapping em : mappings.values()) {
> > > String keyType = em.keyType();
> > >
> > > if (processedKeyTypes.contains(keyType))
> > > continue;
> > >
> > > processedKeyTypes.add(keyType);
> > >
> > > .....
> > >
> > > I think we must add only binary mapping for user pojo if binary
> > marshaller
> > > enable.
> > >
> >
>
Re: CachePojoStore data loading with binary marshaller
Posted by Dmitriy Govorukhin <dm...@gmail.com>.
Hi Valentin,
Other word, are you agree with me? My point that we always must use binary
representation if binary marshaller enable.
On Wed, Apr 12, 2017 at 10:24 AM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> Hi Dmitry,
>
> I can't advise a lot on implementation details, but generally I don't think
> that CacheJdbcPojoStore should ever work with POJO classes in case binary
> marshaller is enabled. It should always work directly with binary objects
> even if POJOs are on server classpath, and switch to POJOs only when other
> marshaller is used.
>
> Makes sense?
>
> -Val
>
> On Tue, Apr 11, 2017 at 12:06 PM, Dmitriy Govorukhin <
> dmitriy.govorukhin@gmail.com> wrote:
>
> > Hi all,
> >
> > I found problem related to CacheJdbcPojoStore. If try to load data, with
> > enabled binary marshaller,
> > error may occur. Problem in entryMappings (this map helps resolve stored
> > type when data loading).
> > If binary marshaller disable we just put entry with java type in this
> map,
> > if enable we put java type + binary type.
> >
> > ......
> >
> > checkTypeConfiguration(cacheName, valKind, valType,
> > type.getValueFields());
> >
> > entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> type,
> > keyKind, valKind, sqlEscapeAll));
> >
> > // Add one more binding to binary typeId for POJOs,
> > // because object could be passed to store in binary format.
> > if (binarySupported && keyKind == TypeKind.POJO) {
> > keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
> >
> > valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
> >
> > entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> > type, TypeKind.BINARY, valKind, sqlEscapeAll));
> > }
> >
> > .......
> >
> >
> > I think it is wrong, because after we use this map like iterator for
> load,
> > and final result dependent from which mapping will be first java or
> binary.
> >
> > Collection<String> processedKeyTypes = new HashSet<>();
> >
> > for (EntryMapping em : mappings.values()) {
> > String keyType = em.keyType();
> >
> > if (processedKeyTypes.contains(keyType))
> > continue;
> >
> > processedKeyTypes.add(keyType);
> >
> > .....
> >
> > I think we must add only binary mapping for user pojo if binary
> marshaller
> > enable.
> >
>
Re: CachePojoStore data loading with binary marshaller
Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Thu, Apr 13, 2017 at 12:16 AM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> It's not illegal in general, but it doesn't make sense for out of the box
> implementation of cache store.\
>
Ouch. I didn't realize that we deserialize in our own cache store. Any
reason for this behavior? Can this be fixed before the release?
Re: CachePojoStore data loading with binary marshaller
Posted by Valentin Kulichenko <va...@gmail.com>.
It's not illegal in general, but it doesn't make sense for out of the box
implementation of cache store.
-Val
On Wed, Apr 12, 2017 at 11:27 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:
> On Wed, Apr 12, 2017 at 12:24 AM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
>
> > Hi Dmitry,
> >
> > I can't advise a lot on implementation details, but generally I don't
> think
> > that CacheJdbcPojoStore should ever work with POJO classes in case binary
> > marshaller is enabled. It should always work directly with binary objects
> > even if POJOs are on server classpath, and switch to POJOs only when
> other
> > marshaller is used.
> >
> > Makes sense?
> >
>
> Valentin, this makes sense from the performance stand point. However, why
> should it be illegal to deserialize a binary object into class, if the
> class it on the classpath?
>
Re: CachePojoStore data loading with binary marshaller
Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Wed, Apr 12, 2017 at 12:24 AM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> Hi Dmitry,
>
> I can't advise a lot on implementation details, but generally I don't think
> that CacheJdbcPojoStore should ever work with POJO classes in case binary
> marshaller is enabled. It should always work directly with binary objects
> even if POJOs are on server classpath, and switch to POJOs only when other
> marshaller is used.
>
> Makes sense?
>
Valentin, this makes sense from the performance stand point. However, why
should it be illegal to deserialize a binary object into class, if the
class it on the classpath?
Re: CachePojoStore data loading with binary marshaller
Posted by Valentin Kulichenko <va...@gmail.com>.
Hi Dmitry,
I can't advise a lot on implementation details, but generally I don't think
that CacheJdbcPojoStore should ever work with POJO classes in case binary
marshaller is enabled. It should always work directly with binary objects
even if POJOs are on server classpath, and switch to POJOs only when other
marshaller is used.
Makes sense?
-Val
On Tue, Apr 11, 2017 at 12:06 PM, Dmitriy Govorukhin <
dmitriy.govorukhin@gmail.com> wrote:
> Hi all,
>
> I found problem related to CacheJdbcPojoStore. If try to load data, with
> enabled binary marshaller,
> error may occur. Problem in entryMappings (this map helps resolve stored
> type when data loading).
> If binary marshaller disable we just put entry with java type in this map,
> if enable we put java type + binary type.
>
> ......
>
> checkTypeConfiguration(cacheName, valKind, valType,
> type.getValueFields());
>
> entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect, type,
> keyKind, valKind, sqlEscapeAll));
>
> // Add one more binding to binary typeId for POJOs,
> // because object could be passed to store in binary format.
> if (binarySupported && keyKind == TypeKind.POJO) {
> keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
>
> valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
>
> entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect,
> type, TypeKind.BINARY, valKind, sqlEscapeAll));
> }
>
> .......
>
>
> I think it is wrong, because after we use this map like iterator for load,
> and final result dependent from which mapping will be first java or binary.
>
> Collection<String> processedKeyTypes = new HashSet<>();
>
> for (EntryMapping em : mappings.values()) {
> String keyType = em.keyType();
>
> if (processedKeyTypes.contains(keyType))
> continue;
>
> processedKeyTypes.add(keyType);
>
> .....
>
> I think we must add only binary mapping for user pojo if binary marshaller
> enable.
>