You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Nikolay Izhikov <ni...@apache.org> on 2021/04/30 13:31:13 UTC
[DISCUSSION] Array to BinaryObject serialization
Hello, Igniters.
Currently, binary marshaller works as follows(Say, we have a class `User` then):
IgniteBinary#toBinary(User)` -> BinaryObject
IgniteBinary#toBinary(User[])` -> Object[]
IgniteBinary#toBinary(Object[])` -> Object[]
This means, that we lose array component type information during binary serialization.
AFAIK, it’s a design choice made during binary infrastructure development.
This lead to the following disadvantages:
1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
2. Ignite internals(service grid, .Net calls) contains many tweaks and hacks to deal with custom user array and still has many issues [1]
I propose to make breaking changes and fix the custom user array SeDe as follows:
1. Implement binary serialization that correctly Ser and Deser array using some kind of the wrapper (BinaryArrayWrapper).
IgniteBinary#toBinary(User)` -> BinaryObject
IgniteBinary#toBinary(User[])` -> BinaryObject
IgniteBinary#toBinary(Object[])` -> BinaryObject
2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables correct SerDe of arrays. The default value is false to keep backward compatibility in the next Ignite release(2.11).
3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite releases (2.12).
WDYT?
[1] https://issues.apache.org/jira/browse/IGNITE-14299
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Hello, Igniters.
I've prepared implementation of support of BinaryArray.
Please, join the review.
Ticket - https://issues.apache.org/jira/browse/IGNITE-14742
PR - https://github.com/apache/ignite/pull/9490
ср, 19 мая 2021 г. в 14:44, Ilya Kasnacheev <il...@gmail.com>:
> Hello!
>
> We don't have to do it in toBinary() - we may preserve its behavior if you
> like, and only do that transition for Cache API, where additional support
> may be added to
> e.g.
> org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 мая 2021 г. в 14:23, Nikolay Izhikov <ni...@apache.org>:
>
> > Ilya.
> >
> > Actually, current behaviour described even in documentation [1]
> >
> >
> > > Note that not all objects are converted to the binary object format.
> The
> > following classes are never converted (e.g., the toBinary(Object) method
> > returns the original object, and instances of these classes are stored
> > without changes):
> > ...
> > > ... arrays of objects (but the objects inside them are reconverted if
> > they are binary)
> >
> > [1]
> >
> https://ignite.apache.org/docs/latest/key-value-api/binary-objects#enabling-binary-mode-for-caches
> >
> > > 19 мая 2021 г., в 14:12, Ilya Kasnacheev <il...@gmail.com>
> > написал(а):
> > >
> > > Hello!
> > >
> > > Why do you need to take compatibility into account here at all?
> > > You can start writing entries to cache in new format while reading both
> > old
> > > and new format.
> > >
> > > I consider returning Object[] instead of ConcreteType[] a bug so it's
> > > totally OK to start returning the "better" ConcreteType[] instead.
> > >
> > > I think you can just fix the bug while preserving accessibility of old
> > > (pre-bugfix) data as it was pre-2.11.
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> > >
> > >
> > > ср, 19 мая 2021 г. в 14:08, Nikolay Izhikov <ni...@apache.org>:
> > >
> > >> Ilya,
> > >>
> > >>> Maybe we should just automate that, e.g., whenever user stores
> Type[],
> > >> always store one-field ArrayWrapper object, and automatically unwrap
> it
> > on
> > >> get()
> > >>
> > >> Yes, I’m talking about this opportunity from the beginning of the
> > thread.
> > >>
> > >>> 1. Implement binary serialization that correctly Ser and Deser array
> > >> using some kind of the wrapper (BinaryArrayWrapper).
> > >>
> > >>
> > >>> 19 мая 2021 г., в 14:05, Ilya Kasnacheev <il...@gmail.com>
> > >> написал(а):
> > >>>
> > >>> Hello!
> > >>>
> > >>> Yes, it does not look pretty, I agree. But I'm saying that I've not
> > >>> encountered this issue on the user list before, and it can probably
> be
> > >>> easily countered by storing some one-field ArrayWrapper object in the
> > >> cache
> > >>> as value.
> > >>>
> > >>> Maybe we should just automate that, e.g., whenever user stores
> Type[],
> > >>> always store one-field ArrayWrapper object, and automatically unwrap
> it
> > >> on
> > >>> get()
> > >>> This way we may not even need any changes to storage format, if
> binary
> > >>> marshaller does not mind.
> > >>>
> > >>> Regards,
> > >>>
> > >>> Regards,
> > >>> --
> > >>> Ilya Kasnacheev
> > >>>
> > >>>
> > >>> ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
> > >>>
> > >>>> Igniters.
> > >>>>
> > >>>> Just to clarify the issue:
> > >>>>
> > >>>> ```
> > >>>> public class BinaryObjectTest extends GridCommonAbstractTest {
> > >>>> /** */
> > >>>> @Test
> > >>>> public void testArray() throws Exception {
> > >>>> Ignite ign = startGrid();
> > >>>>
> > >>>> IgniteCache<Integer, TestClass1[]> cache =
> > >>>> ign.createCache("my-cache");
> > >>>>
> > >>>> cache.put(1, new TestClass1[] {new TestClass1(), new
> > >>>> TestClass1()});
> > >>>> TestClass1[] obj = cache.get(1); //This will fail with
> > >>>> ClassCastException.
> > >>>>
> > >>>> assertEquals(TestClass1[].class, obj.getClass());
> > >>>> }
> > >>>> }
> > >>>> ```
> > >>>>
> > >>>>> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
> > >>>> написал(а):
> > >>>>>
> > >>>>> Thanks, Ilya.
> > >>>>>
> > >>>>> Can you put more context on this?
> > >>>>> I don’t familiar with these issues.
> > >>>>>
> > >>>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com
> > >
> > >>>> написал(а):
> > >>>>>>
> > >>>>>> Hello!
> > >>>>>>
> > >>>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary
> > Object
> > >>>>>> Fields.
> > >>>>>>
> > >>>>>> Regards,
> > >>>>>> --
> > >>>>>> Ilya Kasnacheev
> > >>>>>>
> > >>>>>>
> > >>>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <nizhikov@apache.org
> >:
> > >>>>>>
> > >>>>>>> Hello,
> > >>>>>>>
> > >>>>>>>> However, for internal platform and services implementations we
> > >> should
> > >>>>>>> fix the root cause:
> > >>>>>>>> avoid extra deserialization->serialization pass completely.
> > >>>>>>>> This will also improve performance.
> > >>>>>>>
> > >>>>>>> Pavel, thanks for the feedback.
> > >>>>>>> If I understand correctly, your suggestion is to know data size
> at
> > >> the
> > >>>>>>> start of reading service parameters.
> > >>>>>>> Is it correct?
> > >>>>>>>
> > >>>>>>> Right now, when the service method invoked we pass an array of
> > >>>> parameters
> > >>>>>>> through platform reader/writer machinery.
> > >>>>>>> On java side parameters read one by one and we don’t know the
> size
> > of
> > >>>> the
> > >>>>>>> data on the read start.
> > >>>>>>>
> > >>>>>>> AFAICU, this will require x2 memory on the .Net or thin
> > client-side.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>
> > >>
> >
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> if we are to break compatibility, I would like to see it done
> for
> > >> some
> > >>>>>>> really common pain point.
> > >>>>>>>
> > >>>>>>> Ilya, can you, please, provide a list of common issues with
> Ignite
> > >> that
> > >>>>>>> can be resolved
> > >>>>>>> only with compatibility breakage?
> > >>>>>>>
> > >>>>>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <
> > ilya.kasnacheev@gmail.com>
> > >>>>>>> написал(а):
> > >>>>>>>>
> > >>>>>>>> Hello!
> > >>>>>>>>
> > >>>>>>>> If we really decide to break some compatibility then Array to
> > >>>>>>> BinaryObject
> > >>>>>>>> serialization will be very, very low on my personal list.
> > >>>>>>>>
> > >>>>>>>> I just don't see how this issue is relevant. I have been reading
> > and
> > >>>>>>>> answering user list for a few years now, and I don't remember a
> > >> single
> > >>>>>>>> question about storing ConcreteType[] as value and complaining
> > about
> > >>>> type
> > >>>>>>>> information loss.
> > >>>>>>>>
> > >>>>>>>> If you have a good scenario how do we keep persistent store
> binary
> > >>>>>>>> compatibility here, without adding a lot of new code and still
> > >>>> checking
> > >>>>>>> for
> > >>>>>>>> both old and new approaches - you can go forward for sure.
> > >>>>>>>>
> > >>>>>>>> However, it does seem questionable where we have a new wrapper
> > class
> > >>>>>>>> specifically for top level arrays. You can have this wrapper in
> > your
> > >>>> own
> > >>>>>>>> client code and it should work OK.
> > >>>>>>>>
> > >>>>>>>> Bottom line, if we are to break compatibility, I would like to
> see
> > >> it
> > >>>>>>> done
> > >>>>>>>> for some really common pain point.
> > >>>>>>>>
> > >>>>>>>> Regards,
> > >>>>>>>> --
> > >>>>>>>> Ilya Kasnacheev
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <
> nizhikov@apache.org
> > >:
> > >>>>>>>>
> > >>>>>>>>> Hello, Ilya.
> > >>>>>>>>>
> > >>>>>>>>> Thanks for the feedback!
> > >>>>>>>>>
> > >>>>>>>>>> For me it sounds like something we would like to do in 3.0
> > >>>>>>>>>
> > >>>>>>>>> Ignite 3 is a very long way to go, so I prefer to target this
> fix
> > >> in
> > >>>>>>>>> Ignite 2.x.
> > >>>>>>>>>
> > >>>>>>>>>> I think making it default "true" is a breaking change and is
> not
> > >>>>>>>>> possible in a minor release
> > >>>>>>>>>
> > >>>>>>>>> Yes, you are correct it is a breaking change.
> > >>>>>>>>> It seems for me, we all agreed that breaking changes are
> possible
> > >> in
> > >>>>>>> minor
> > >>>>>>>>> releases.
> > >>>>>>>>>
> > >>>>>>>>> Anyway, if we will decide do not to enable this feature by
> > default
> > >>>> it’s
> > >>>>>>> OK
> > >>>>>>>>> for me.
> > >>>>>>>>> We still can implement it and improve the binary SerDe
> mechanism.
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> > >>>> ilya.kasnacheev@gmail.com>
> > >>>>>>>>> написал(а):
> > >>>>>>>>>>
> > >>>>>>>>>> Hello!
> > >>>>>>>>>>
> > >>>>>>>>>> For me it sounds like something we would like to do in 3.0 (if
> > >>>> indeed
> > >>>>>>> it
> > >>>>>>>>>> will have arrays as possible value (or key) type), but doing
> it
> > in
> > >>>> 2.x
> > >>>>>>>>>> raises concerns whether it has enough time left to stabilize.
> > >>>>>>>>>>
> > >>>>>>>>>> Also, I think making it default "true" is a breaking change
> and
> > is
> > >>>> not
> > >>>>>>>>>> possible in a minor release, case in point,
> > >>>>>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and
> it
> > >> is
> > >>>>>>> less
> > >>>>>>>>>> destructive.
> > >>>>>>>>>>
> > >>>>>>>>>> Of course I would also like to hear what other community
> members
> > >>>> think.
> > >>>>>>>>>>
> > >>>>>>>>>> Regards,
> > >>>>>>>>>> --
> > >>>>>>>>>> Ilya Kasnacheev
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <
> > nizhikov@apache.org
> > >>> :
> > >>>>>>>>>>
> > >>>>>>>>>>> Igniters,
> > >>>>>>>>>>>
> > >>>>>>>>>>> Want to clarify my proposal about new array store format.
> > >>>>>>>>>>> I think we should store array in special binary wrapper that
> > will
> > >>>> keep
> > >>>>>>>>>>> original component type
> > >>>>>>>>>>>
> > >>>>>>>>>>> ```
> > >>>>>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> > >>>>>>>>> Externalizable {
> > >>>>>>>>>>> /** Type ID. */
> > >>>>>>>>>>> private int compTypeId;
> > >>>>>>>>>>>
> > >>>>>>>>>>> /** Raw data. */
> > >>>>>>>>>>> private String compClsName;
> > >>>>>>>>>>>
> > >>>>>>>>>>> /** Value. */
> > >>>>>>>>>>> private Object[] arr;
> > >>>>>>>>>>>
> > >>>>>>>>>>> // Further implementation.
> > >>>>>>>>>>> }
> > >>>>>>>>>>> ```
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <
> > >> nizhikov.dev@gmail.com
> > >>>>>
> > >>>>>>>>>>> написал(а):
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Hello, Igniters.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Currently, binary marshaller works as follows(Say, we have a
> > >> class
> > >>>>>>>>>>> `User` then):
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
> > >>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> This means, that we lose array component type information
> > during
> > >>>>>>> binary
> > >>>>>>>>>>> serialization.
> > >>>>>>>>>>>> AFAIK, it’s a design choice made during binary
> infrastructure
> > >>>>>>>>>>> development.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> This lead to the following disadvantages:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe
> > mechanism.
> > >>>>>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many
> > >> tweaks
> > >>>>>>> and
> > >>>>>>>>>>> hacks to deal with custom user array and still has many
> issues
> > >> [1]
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> I propose to make breaking changes and fix the custom user
> > array
> > >>>> SeDe
> > >>>>>>>>> as
> > >>>>>>>>>>> follows:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 1. Implement binary serialization that correctly Ser and
> Deser
> > >>>>>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> > >>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that
> > enables
> > >>>>>>>>>>> correct SerDe of arrays. The default value is false to keep
> > >>>> backward
> > >>>>>>>>>>> compatibility in the next Ignite release(2.11).
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> > >>>> Ignite
> > >>>>>>>>>>> releases (2.12).
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> WDYT?
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>
> > >>
> >
> >
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
We don't have to do it in toBinary() - we may preserve its behavior if you
like, and only do that transition for Cache API, where additional support
may be added to
e.g. org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl
Regards,
--
Ilya Kasnacheev
ср, 19 мая 2021 г. в 14:23, Nikolay Izhikov <ni...@apache.org>:
> Ilya.
>
> Actually, current behaviour described even in documentation [1]
>
>
> > Note that not all objects are converted to the binary object format. The
> following classes are never converted (e.g., the toBinary(Object) method
> returns the original object, and instances of these classes are stored
> without changes):
> ...
> > ... arrays of objects (but the objects inside them are reconverted if
> they are binary)
>
> [1]
> https://ignite.apache.org/docs/latest/key-value-api/binary-objects#enabling-binary-mode-for-caches
>
> > 19 мая 2021 г., в 14:12, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > Why do you need to take compatibility into account here at all?
> > You can start writing entries to cache in new format while reading both
> old
> > and new format.
> >
> > I consider returning Object[] instead of ConcreteType[] a bug so it's
> > totally OK to start returning the "better" ConcreteType[] instead.
> >
> > I think you can just fix the bug while preserving accessibility of old
> > (pre-bugfix) data as it was pre-2.11.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 19 мая 2021 г. в 14:08, Nikolay Izhikov <ni...@apache.org>:
> >
> >> Ilya,
> >>
> >>> Maybe we should just automate that, e.g., whenever user stores Type[],
> >> always store one-field ArrayWrapper object, and automatically unwrap it
> on
> >> get()
> >>
> >> Yes, I’m talking about this opportunity from the beginning of the
> thread.
> >>
> >>> 1. Implement binary serialization that correctly Ser and Deser array
> >> using some kind of the wrapper (BinaryArrayWrapper).
> >>
> >>
> >>> 19 мая 2021 г., в 14:05, Ilya Kasnacheev <il...@gmail.com>
> >> написал(а):
> >>>
> >>> Hello!
> >>>
> >>> Yes, it does not look pretty, I agree. But I'm saying that I've not
> >>> encountered this issue on the user list before, and it can probably be
> >>> easily countered by storing some one-field ArrayWrapper object in the
> >> cache
> >>> as value.
> >>>
> >>> Maybe we should just automate that, e.g., whenever user stores Type[],
> >>> always store one-field ArrayWrapper object, and automatically unwrap it
> >> on
> >>> get()
> >>> This way we may not even need any changes to storage format, if binary
> >>> marshaller does not mind.
> >>>
> >>> Regards,
> >>>
> >>> Regards,
> >>> --
> >>> Ilya Kasnacheev
> >>>
> >>>
> >>> ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
> >>>
> >>>> Igniters.
> >>>>
> >>>> Just to clarify the issue:
> >>>>
> >>>> ```
> >>>> public class BinaryObjectTest extends GridCommonAbstractTest {
> >>>> /** */
> >>>> @Test
> >>>> public void testArray() throws Exception {
> >>>> Ignite ign = startGrid();
> >>>>
> >>>> IgniteCache<Integer, TestClass1[]> cache =
> >>>> ign.createCache("my-cache");
> >>>>
> >>>> cache.put(1, new TestClass1[] {new TestClass1(), new
> >>>> TestClass1()});
> >>>> TestClass1[] obj = cache.get(1); //This will fail with
> >>>> ClassCastException.
> >>>>
> >>>> assertEquals(TestClass1[].class, obj.getClass());
> >>>> }
> >>>> }
> >>>> ```
> >>>>
> >>>>> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
> >>>> написал(а):
> >>>>>
> >>>>> Thanks, Ilya.
> >>>>>
> >>>>> Can you put more context on this?
> >>>>> I don’t familiar with these issues.
> >>>>>
> >>>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <ilya.kasnacheev@gmail.com
> >
> >>>> написал(а):
> >>>>>>
> >>>>>> Hello!
> >>>>>>
> >>>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary
> Object
> >>>>>> Fields.
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Ilya Kasnacheev
> >>>>>>
> >>>>>>
> >>>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
> >>>>>>
> >>>>>>> Hello,
> >>>>>>>
> >>>>>>>> However, for internal platform and services implementations we
> >> should
> >>>>>>> fix the root cause:
> >>>>>>>> avoid extra deserialization->serialization pass completely.
> >>>>>>>> This will also improve performance.
> >>>>>>>
> >>>>>>> Pavel, thanks for the feedback.
> >>>>>>> If I understand correctly, your suggestion is to know data size at
> >> the
> >>>>>>> start of reading service parameters.
> >>>>>>> Is it correct?
> >>>>>>>
> >>>>>>> Right now, when the service method invoked we pass an array of
> >>>> parameters
> >>>>>>> through platform reader/writer machinery.
> >>>>>>> On java side parameters read one by one and we don’t know the size
> of
> >>>> the
> >>>>>>> data on the read start.
> >>>>>>>
> >>>>>>> AFAICU, this will require x2 memory on the .Net or thin
> client-side.
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>
> >>
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> >>>>>>>
> >>>>>>>
> >>>>>>>> if we are to break compatibility, I would like to see it done for
> >> some
> >>>>>>> really common pain point.
> >>>>>>>
> >>>>>>> Ilya, can you, please, provide a list of common issues with Ignite
> >> that
> >>>>>>> can be resolved
> >>>>>>> only with compatibility breakage?
> >>>>>>>
> >>>>>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com>
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> Hello!
> >>>>>>>>
> >>>>>>>> If we really decide to break some compatibility then Array to
> >>>>>>> BinaryObject
> >>>>>>>> serialization will be very, very low on my personal list.
> >>>>>>>>
> >>>>>>>> I just don't see how this issue is relevant. I have been reading
> and
> >>>>>>>> answering user list for a few years now, and I don't remember a
> >> single
> >>>>>>>> question about storing ConcreteType[] as value and complaining
> about
> >>>> type
> >>>>>>>> information loss.
> >>>>>>>>
> >>>>>>>> If you have a good scenario how do we keep persistent store binary
> >>>>>>>> compatibility here, without adding a lot of new code and still
> >>>> checking
> >>>>>>> for
> >>>>>>>> both old and new approaches - you can go forward for sure.
> >>>>>>>>
> >>>>>>>> However, it does seem questionable where we have a new wrapper
> class
> >>>>>>>> specifically for top level arrays. You can have this wrapper in
> your
> >>>> own
> >>>>>>>> client code and it should work OK.
> >>>>>>>>
> >>>>>>>> Bottom line, if we are to break compatibility, I would like to see
> >> it
> >>>>>>> done
> >>>>>>>> for some really common pain point.
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> --
> >>>>>>>> Ilya Kasnacheev
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <nizhikov@apache.org
> >:
> >>>>>>>>
> >>>>>>>>> Hello, Ilya.
> >>>>>>>>>
> >>>>>>>>> Thanks for the feedback!
> >>>>>>>>>
> >>>>>>>>>> For me it sounds like something we would like to do in 3.0
> >>>>>>>>>
> >>>>>>>>> Ignite 3 is a very long way to go, so I prefer to target this fix
> >> in
> >>>>>>>>> Ignite 2.x.
> >>>>>>>>>
> >>>>>>>>>> I think making it default "true" is a breaking change and is not
> >>>>>>>>> possible in a minor release
> >>>>>>>>>
> >>>>>>>>> Yes, you are correct it is a breaking change.
> >>>>>>>>> It seems for me, we all agreed that breaking changes are possible
> >> in
> >>>>>>> minor
> >>>>>>>>> releases.
> >>>>>>>>>
> >>>>>>>>> Anyway, if we will decide do not to enable this feature by
> default
> >>>> it’s
> >>>>>>> OK
> >>>>>>>>> for me.
> >>>>>>>>> We still can implement it and improve the binary SerDe mechanism.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> >>>> ilya.kasnacheev@gmail.com>
> >>>>>>>>> написал(а):
> >>>>>>>>>>
> >>>>>>>>>> Hello!
> >>>>>>>>>>
> >>>>>>>>>> For me it sounds like something we would like to do in 3.0 (if
> >>>> indeed
> >>>>>>> it
> >>>>>>>>>> will have arrays as possible value (or key) type), but doing it
> in
> >>>> 2.x
> >>>>>>>>>> raises concerns whether it has enough time left to stabilize.
> >>>>>>>>>>
> >>>>>>>>>> Also, I think making it default "true" is a breaking change and
> is
> >>>> not
> >>>>>>>>>> possible in a minor release, case in point,
> >>>>>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it
> >> is
> >>>>>>> less
> >>>>>>>>>> destructive.
> >>>>>>>>>>
> >>>>>>>>>> Of course I would also like to hear what other community members
> >>>> think.
> >>>>>>>>>>
> >>>>>>>>>> Regards,
> >>>>>>>>>> --
> >>>>>>>>>> Ilya Kasnacheev
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <
> nizhikov@apache.org
> >>> :
> >>>>>>>>>>
> >>>>>>>>>>> Igniters,
> >>>>>>>>>>>
> >>>>>>>>>>> Want to clarify my proposal about new array store format.
> >>>>>>>>>>> I think we should store array in special binary wrapper that
> will
> >>>> keep
> >>>>>>>>>>> original component type
> >>>>>>>>>>>
> >>>>>>>>>>> ```
> >>>>>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> >>>>>>>>> Externalizable {
> >>>>>>>>>>> /** Type ID. */
> >>>>>>>>>>> private int compTypeId;
> >>>>>>>>>>>
> >>>>>>>>>>> /** Raw data. */
> >>>>>>>>>>> private String compClsName;
> >>>>>>>>>>>
> >>>>>>>>>>> /** Value. */
> >>>>>>>>>>> private Object[] arr;
> >>>>>>>>>>>
> >>>>>>>>>>> // Further implementation.
> >>>>>>>>>>> }
> >>>>>>>>>>> ```
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <
> >> nizhikov.dev@gmail.com
> >>>>>
> >>>>>>>>>>> написал(а):
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hello, Igniters.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Currently, binary marshaller works as follows(Say, we have a
> >> class
> >>>>>>>>>>> `User` then):
> >>>>>>>>>>>>
> >>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
> >>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>>>>>>>>>>
> >>>>>>>>>>>> This means, that we lose array component type information
> during
> >>>>>>> binary
> >>>>>>>>>>> serialization.
> >>>>>>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
> >>>>>>>>>>> development.
> >>>>>>>>>>>>
> >>>>>>>>>>>> This lead to the following disadvantages:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe
> mechanism.
> >>>>>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many
> >> tweaks
> >>>>>>> and
> >>>>>>>>>>> hacks to deal with custom user array and still has many issues
> >> [1]
> >>>>>>>>>>>>
> >>>>>>>>>>>> I propose to make breaking changes and fix the custom user
> array
> >>>> SeDe
> >>>>>>>>> as
> >>>>>>>>>>> follows:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
> >>>>>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>>>>>>>>>>
> >>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>>>>>>>>>>
> >>>>>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that
> enables
> >>>>>>>>>>> correct SerDe of arrays. The default value is false to keep
> >>>> backward
> >>>>>>>>>>> compatibility in the next Ignite release(2.11).
> >>>>>>>>>>>>
> >>>>>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> >>>> Ignite
> >>>>>>>>>>> releases (2.12).
> >>>>>>>>>>>>
> >>>>>>>>>>>> WDYT?
> >>>>>>>>>>>>
> >>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Ilya.
Actually, current behaviour described even in documentation [1]
> Note that not all objects are converted to the binary object format. The following classes are never converted (e.g., the toBinary(Object) method returns the original object, and instances of these classes are stored without changes):
...
> ... arrays of objects (but the objects inside them are reconverted if they are binary)
[1] https://ignite.apache.org/docs/latest/key-value-api/binary-objects#enabling-binary-mode-for-caches
> 19 мая 2021 г., в 14:12, Ilya Kasnacheev <il...@gmail.com> написал(а):
>
> Hello!
>
> Why do you need to take compatibility into account here at all?
> You can start writing entries to cache in new format while reading both old
> and new format.
>
> I consider returning Object[] instead of ConcreteType[] a bug so it's
> totally OK to start returning the "better" ConcreteType[] instead.
>
> I think you can just fix the bug while preserving accessibility of old
> (pre-bugfix) data as it was pre-2.11.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 мая 2021 г. в 14:08, Nikolay Izhikov <ni...@apache.org>:
>
>> Ilya,
>>
>>> Maybe we should just automate that, e.g., whenever user stores Type[],
>> always store one-field ArrayWrapper object, and automatically unwrap it on
>> get()
>>
>> Yes, I’m talking about this opportunity from the beginning of the thread.
>>
>>> 1. Implement binary serialization that correctly Ser and Deser array
>> using some kind of the wrapper (BinaryArrayWrapper).
>>
>>
>>> 19 мая 2021 г., в 14:05, Ilya Kasnacheev <il...@gmail.com>
>> написал(а):
>>>
>>> Hello!
>>>
>>> Yes, it does not look pretty, I agree. But I'm saying that I've not
>>> encountered this issue on the user list before, and it can probably be
>>> easily countered by storing some one-field ArrayWrapper object in the
>> cache
>>> as value.
>>>
>>> Maybe we should just automate that, e.g., whenever user stores Type[],
>>> always store one-field ArrayWrapper object, and automatically unwrap it
>> on
>>> get()
>>> This way we may not even need any changes to storage format, if binary
>>> marshaller does not mind.
>>>
>>> Regards,
>>>
>>> Regards,
>>> --
>>> Ilya Kasnacheev
>>>
>>>
>>> ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
>>>
>>>> Igniters.
>>>>
>>>> Just to clarify the issue:
>>>>
>>>> ```
>>>> public class BinaryObjectTest extends GridCommonAbstractTest {
>>>> /** */
>>>> @Test
>>>> public void testArray() throws Exception {
>>>> Ignite ign = startGrid();
>>>>
>>>> IgniteCache<Integer, TestClass1[]> cache =
>>>> ign.createCache("my-cache");
>>>>
>>>> cache.put(1, new TestClass1[] {new TestClass1(), new
>>>> TestClass1()});
>>>> TestClass1[] obj = cache.get(1); //This will fail with
>>>> ClassCastException.
>>>>
>>>> assertEquals(TestClass1[].class, obj.getClass());
>>>> }
>>>> }
>>>> ```
>>>>
>>>>> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
>>>> написал(а):
>>>>>
>>>>> Thanks, Ilya.
>>>>>
>>>>> Can you put more context on this?
>>>>> I don’t familiar with these issues.
>>>>>
>>>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com>
>>>> написал(а):
>>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>>>>>> Fields.
>>>>>>
>>>>>> Regards,
>>>>>> --
>>>>>> Ilya Kasnacheev
>>>>>>
>>>>>>
>>>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>>> However, for internal platform and services implementations we
>> should
>>>>>>> fix the root cause:
>>>>>>>> avoid extra deserialization->serialization pass completely.
>>>>>>>> This will also improve performance.
>>>>>>>
>>>>>>> Pavel, thanks for the feedback.
>>>>>>> If I understand correctly, your suggestion is to know data size at
>> the
>>>>>>> start of reading service parameters.
>>>>>>> Is it correct?
>>>>>>>
>>>>>>> Right now, when the service method invoked we pass an array of
>>>> parameters
>>>>>>> through platform reader/writer machinery.
>>>>>>> On java side parameters read one by one and we don’t know the size of
>>>> the
>>>>>>> data on the read start.
>>>>>>>
>>>>>>> AFAICU, this will require x2 memory on the .Net or thin client-side.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
>>>>>>>
>>>>>>>
>>>>>>>> if we are to break compatibility, I would like to see it done for
>> some
>>>>>>> really common pain point.
>>>>>>>
>>>>>>> Ilya, can you, please, provide a list of common issues with Ignite
>> that
>>>>>>> can be resolved
>>>>>>> only with compatibility breakage?
>>>>>>>
>>>>>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> Hello!
>>>>>>>>
>>>>>>>> If we really decide to break some compatibility then Array to
>>>>>>> BinaryObject
>>>>>>>> serialization will be very, very low on my personal list.
>>>>>>>>
>>>>>>>> I just don't see how this issue is relevant. I have been reading and
>>>>>>>> answering user list for a few years now, and I don't remember a
>> single
>>>>>>>> question about storing ConcreteType[] as value and complaining about
>>>> type
>>>>>>>> information loss.
>>>>>>>>
>>>>>>>> If you have a good scenario how do we keep persistent store binary
>>>>>>>> compatibility here, without adding a lot of new code and still
>>>> checking
>>>>>>> for
>>>>>>>> both old and new approaches - you can go forward for sure.
>>>>>>>>
>>>>>>>> However, it does seem questionable where we have a new wrapper class
>>>>>>>> specifically for top level arrays. You can have this wrapper in your
>>>> own
>>>>>>>> client code and it should work OK.
>>>>>>>>
>>>>>>>> Bottom line, if we are to break compatibility, I would like to see
>> it
>>>>>>> done
>>>>>>>> for some really common pain point.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> --
>>>>>>>> Ilya Kasnacheev
>>>>>>>>
>>>>>>>>
>>>>>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
>>>>>>>>
>>>>>>>>> Hello, Ilya.
>>>>>>>>>
>>>>>>>>> Thanks for the feedback!
>>>>>>>>>
>>>>>>>>>> For me it sounds like something we would like to do in 3.0
>>>>>>>>>
>>>>>>>>> Ignite 3 is a very long way to go, so I prefer to target this fix
>> in
>>>>>>>>> Ignite 2.x.
>>>>>>>>>
>>>>>>>>>> I think making it default "true" is a breaking change and is not
>>>>>>>>> possible in a minor release
>>>>>>>>>
>>>>>>>>> Yes, you are correct it is a breaking change.
>>>>>>>>> It seems for me, we all agreed that breaking changes are possible
>> in
>>>>>>> minor
>>>>>>>>> releases.
>>>>>>>>>
>>>>>>>>> Anyway, if we will decide do not to enable this feature by default
>>>> it’s
>>>>>>> OK
>>>>>>>>> for me.
>>>>>>>>> We still can implement it and improve the binary SerDe mechanism.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
>>>> ilya.kasnacheev@gmail.com>
>>>>>>>>> написал(а):
>>>>>>>>>>
>>>>>>>>>> Hello!
>>>>>>>>>>
>>>>>>>>>> For me it sounds like something we would like to do in 3.0 (if
>>>> indeed
>>>>>>> it
>>>>>>>>>> will have arrays as possible value (or key) type), but doing it in
>>>> 2.x
>>>>>>>>>> raises concerns whether it has enough time left to stabilize.
>>>>>>>>>>
>>>>>>>>>> Also, I think making it default "true" is a breaking change and is
>>>> not
>>>>>>>>>> possible in a minor release, case in point,
>>>>>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it
>> is
>>>>>>> less
>>>>>>>>>> destructive.
>>>>>>>>>>
>>>>>>>>>> Of course I would also like to hear what other community members
>>>> think.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> --
>>>>>>>>>> Ilya Kasnacheev
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <nizhikov@apache.org
>>> :
>>>>>>>>>>
>>>>>>>>>>> Igniters,
>>>>>>>>>>>
>>>>>>>>>>> Want to clarify my proposal about new array store format.
>>>>>>>>>>> I think we should store array in special binary wrapper that will
>>>> keep
>>>>>>>>>>> original component type
>>>>>>>>>>>
>>>>>>>>>>> ```
>>>>>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
>>>>>>>>> Externalizable {
>>>>>>>>>>> /** Type ID. */
>>>>>>>>>>> private int compTypeId;
>>>>>>>>>>>
>>>>>>>>>>> /** Raw data. */
>>>>>>>>>>> private String compClsName;
>>>>>>>>>>>
>>>>>>>>>>> /** Value. */
>>>>>>>>>>> private Object[] arr;
>>>>>>>>>>>
>>>>>>>>>>> // Further implementation.
>>>>>>>>>>> }
>>>>>>>>>>> ```
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <
>> nizhikov.dev@gmail.com
>>>>>
>>>>>>>>>>> написал(а):
>>>>>>>>>>>>
>>>>>>>>>>>> Hello, Igniters.
>>>>>>>>>>>>
>>>>>>>>>>>> Currently, binary marshaller works as follows(Say, we have a
>> class
>>>>>>>>>>> `User` then):
>>>>>>>>>>>>
>>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
>>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>>>>>>>>>>
>>>>>>>>>>>> This means, that we lose array component type information during
>>>>>>> binary
>>>>>>>>>>> serialization.
>>>>>>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
>>>>>>>>>>> development.
>>>>>>>>>>>>
>>>>>>>>>>>> This lead to the following disadvantages:
>>>>>>>>>>>>
>>>>>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>>>>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many
>> tweaks
>>>>>>> and
>>>>>>>>>>> hacks to deal with custom user array and still has many issues
>> [1]
>>>>>>>>>>>>
>>>>>>>>>>>> I propose to make breaking changes and fix the custom user array
>>>> SeDe
>>>>>>>>> as
>>>>>>>>>>> follows:
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
>>>>>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>>>>>>>>>>
>>>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>>>>>>>>>>
>>>>>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>>>>>>>>>>> correct SerDe of arrays. The default value is false to keep
>>>> backward
>>>>>>>>>>> compatibility in the next Ignite release(2.11).
>>>>>>>>>>>>
>>>>>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
>>>> Ignite
>>>>>>>>>>> releases (2.12).
>>>>>>>>>>>>
>>>>>>>>>>>> WDYT?
>>>>>>>>>>>>
>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>>
>>
>>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
Why do you need to take compatibility into account here at all?
You can start writing entries to cache in new format while reading both old
and new format.
I consider returning Object[] instead of ConcreteType[] a bug so it's
totally OK to start returning the "better" ConcreteType[] instead.
I think you can just fix the bug while preserving accessibility of old
(pre-bugfix) data as it was pre-2.11.
Regards,
--
Ilya Kasnacheev
ср, 19 мая 2021 г. в 14:08, Nikolay Izhikov <ni...@apache.org>:
> Ilya,
>
> > Maybe we should just automate that, e.g., whenever user stores Type[],
> always store one-field ArrayWrapper object, and automatically unwrap it on
> get()
>
> Yes, I’m talking about this opportunity from the beginning of the thread.
>
> > 1. Implement binary serialization that correctly Ser and Deser array
> using some kind of the wrapper (BinaryArrayWrapper).
>
>
> > 19 мая 2021 г., в 14:05, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > Yes, it does not look pretty, I agree. But I'm saying that I've not
> > encountered this issue on the user list before, and it can probably be
> > easily countered by storing some one-field ArrayWrapper object in the
> cache
> > as value.
> >
> > Maybe we should just automate that, e.g., whenever user stores Type[],
> > always store one-field ArrayWrapper object, and automatically unwrap it
> on
> > get()
> > This way we may not even need any changes to storage format, if binary
> > marshaller does not mind.
> >
> > Regards,
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
> >
> >> Igniters.
> >>
> >> Just to clarify the issue:
> >>
> >> ```
> >> public class BinaryObjectTest extends GridCommonAbstractTest {
> >> /** */
> >> @Test
> >> public void testArray() throws Exception {
> >> Ignite ign = startGrid();
> >>
> >> IgniteCache<Integer, TestClass1[]> cache =
> >> ign.createCache("my-cache");
> >>
> >> cache.put(1, new TestClass1[] {new TestClass1(), new
> >> TestClass1()});
> >> TestClass1[] obj = cache.get(1); //This will fail with
> >> ClassCastException.
> >>
> >> assertEquals(TestClass1[].class, obj.getClass());
> >> }
> >> }
> >> ```
> >>
> >>> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
> >> написал(а):
> >>>
> >>> Thanks, Ilya.
> >>>
> >>> Can you put more context on this?
> >>> I don’t familiar with these issues.
> >>>
> >>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com>
> >> написал(а):
> >>>>
> >>>> Hello!
> >>>>
> >>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> >>>> Fields.
> >>>>
> >>>> Regards,
> >>>> --
> >>>> Ilya Kasnacheev
> >>>>
> >>>>
> >>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>>> However, for internal platform and services implementations we
> should
> >>>>> fix the root cause:
> >>>>>> avoid extra deserialization->serialization pass completely.
> >>>>>> This will also improve performance.
> >>>>>
> >>>>> Pavel, thanks for the feedback.
> >>>>> If I understand correctly, your suggestion is to know data size at
> the
> >>>>> start of reading service parameters.
> >>>>> Is it correct?
> >>>>>
> >>>>> Right now, when the service method invoked we pass an array of
> >> parameters
> >>>>> through platform reader/writer machinery.
> >>>>> On java side parameters read one by one and we don’t know the size of
> >> the
> >>>>> data on the read start.
> >>>>>
> >>>>> AFAICU, this will require x2 memory on the .Net or thin client-side.
> >>>>>
> >>>>>
> >>>>>
> >>
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> >>>>>
> >>>>>
> >>>>>> if we are to break compatibility, I would like to see it done for
> some
> >>>>> really common pain point.
> >>>>>
> >>>>> Ilya, can you, please, provide a list of common issues with Ignite
> that
> >>>>> can be resolved
> >>>>> only with compatibility breakage?
> >>>>>
> >>>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
> >>>>> написал(а):
> >>>>>>
> >>>>>> Hello!
> >>>>>>
> >>>>>> If we really decide to break some compatibility then Array to
> >>>>> BinaryObject
> >>>>>> serialization will be very, very low on my personal list.
> >>>>>>
> >>>>>> I just don't see how this issue is relevant. I have been reading and
> >>>>>> answering user list for a few years now, and I don't remember a
> single
> >>>>>> question about storing ConcreteType[] as value and complaining about
> >> type
> >>>>>> information loss.
> >>>>>>
> >>>>>> If you have a good scenario how do we keep persistent store binary
> >>>>>> compatibility here, without adding a lot of new code and still
> >> checking
> >>>>> for
> >>>>>> both old and new approaches - you can go forward for sure.
> >>>>>>
> >>>>>> However, it does seem questionable where we have a new wrapper class
> >>>>>> specifically for top level arrays. You can have this wrapper in your
> >> own
> >>>>>> client code and it should work OK.
> >>>>>>
> >>>>>> Bottom line, if we are to break compatibility, I would like to see
> it
> >>>>> done
> >>>>>> for some really common pain point.
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Ilya Kasnacheev
> >>>>>>
> >>>>>>
> >>>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
> >>>>>>
> >>>>>>> Hello, Ilya.
> >>>>>>>
> >>>>>>> Thanks for the feedback!
> >>>>>>>
> >>>>>>>> For me it sounds like something we would like to do in 3.0
> >>>>>>>
> >>>>>>> Ignite 3 is a very long way to go, so I prefer to target this fix
> in
> >>>>>>> Ignite 2.x.
> >>>>>>>
> >>>>>>>> I think making it default "true" is a breaking change and is not
> >>>>>>> possible in a minor release
> >>>>>>>
> >>>>>>> Yes, you are correct it is a breaking change.
> >>>>>>> It seems for me, we all agreed that breaking changes are possible
> in
> >>>>> minor
> >>>>>>> releases.
> >>>>>>>
> >>>>>>> Anyway, if we will decide do not to enable this feature by default
> >> it’s
> >>>>> OK
> >>>>>>> for me.
> >>>>>>> We still can implement it and improve the binary SerDe mechanism.
> >>>>>>>
> >>>>>>>
> >>>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> >> ilya.kasnacheev@gmail.com>
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> Hello!
> >>>>>>>>
> >>>>>>>> For me it sounds like something we would like to do in 3.0 (if
> >> indeed
> >>>>> it
> >>>>>>>> will have arrays as possible value (or key) type), but doing it in
> >> 2.x
> >>>>>>>> raises concerns whether it has enough time left to stabilize.
> >>>>>>>>
> >>>>>>>> Also, I think making it default "true" is a breaking change and is
> >> not
> >>>>>>>> possible in a minor release, case in point,
> >>>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it
> is
> >>>>> less
> >>>>>>>> destructive.
> >>>>>>>>
> >>>>>>>> Of course I would also like to hear what other community members
> >> think.
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> --
> >>>>>>>> Ilya Kasnacheev
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <nizhikov@apache.org
> >:
> >>>>>>>>
> >>>>>>>>> Igniters,
> >>>>>>>>>
> >>>>>>>>> Want to clarify my proposal about new array store format.
> >>>>>>>>> I think we should store array in special binary wrapper that will
> >> keep
> >>>>>>>>> original component type
> >>>>>>>>>
> >>>>>>>>> ```
> >>>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> >>>>>>> Externalizable {
> >>>>>>>>> /** Type ID. */
> >>>>>>>>> private int compTypeId;
> >>>>>>>>>
> >>>>>>>>> /** Raw data. */
> >>>>>>>>> private String compClsName;
> >>>>>>>>>
> >>>>>>>>> /** Value. */
> >>>>>>>>> private Object[] arr;
> >>>>>>>>>
> >>>>>>>>> // Further implementation.
> >>>>>>>>> }
> >>>>>>>>> ```
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <
> nizhikov.dev@gmail.com
> >>>
> >>>>>>>>> написал(а):
> >>>>>>>>>>
> >>>>>>>>>> Hello, Igniters.
> >>>>>>>>>>
> >>>>>>>>>> Currently, binary marshaller works as follows(Say, we have a
> class
> >>>>>>>>> `User` then):
> >>>>>>>>>>
> >>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
> >>>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>>>>>>>>
> >>>>>>>>>> This means, that we lose array component type information during
> >>>>> binary
> >>>>>>>>> serialization.
> >>>>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
> >>>>>>>>> development.
> >>>>>>>>>>
> >>>>>>>>>> This lead to the following disadvantages:
> >>>>>>>>>>
> >>>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many
> tweaks
> >>>>> and
> >>>>>>>>> hacks to deal with custom user array and still has many issues
> [1]
> >>>>>>>>>>
> >>>>>>>>>> I propose to make breaking changes and fix the custom user array
> >> SeDe
> >>>>>>> as
> >>>>>>>>> follows:
> >>>>>>>>>>
> >>>>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
> >>>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>>>>>>>>
> >>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>>>>>>>>
> >>>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >>>>>>>>> correct SerDe of arrays. The default value is false to keep
> >> backward
> >>>>>>>>> compatibility in the next Ignite release(2.11).
> >>>>>>>>>>
> >>>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> >> Ignite
> >>>>>>>>> releases (2.12).
> >>>>>>>>>>
> >>>>>>>>>> WDYT?
> >>>>>>>>>>
> >>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Ilya,
> Maybe we should just automate that, e.g., whenever user stores Type[], always store one-field ArrayWrapper object, and automatically unwrap it on get()
Yes, I’m talking about this opportunity from the beginning of the thread.
> 1. Implement binary serialization that correctly Ser and Deser array using some kind of the wrapper (BinaryArrayWrapper).
> 19 мая 2021 г., в 14:05, Ilya Kasnacheev <il...@gmail.com> написал(а):
>
> Hello!
>
> Yes, it does not look pretty, I agree. But I'm saying that I've not
> encountered this issue on the user list before, and it can probably be
> easily countered by storing some one-field ArrayWrapper object in the cache
> as value.
>
> Maybe we should just automate that, e.g., whenever user stores Type[],
> always store one-field ArrayWrapper object, and automatically unwrap it on
> get()
> This way we may not even need any changes to storage format, if binary
> marshaller does not mind.
>
> Regards,
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
>
>> Igniters.
>>
>> Just to clarify the issue:
>>
>> ```
>> public class BinaryObjectTest extends GridCommonAbstractTest {
>> /** */
>> @Test
>> public void testArray() throws Exception {
>> Ignite ign = startGrid();
>>
>> IgniteCache<Integer, TestClass1[]> cache =
>> ign.createCache("my-cache");
>>
>> cache.put(1, new TestClass1[] {new TestClass1(), new
>> TestClass1()});
>> TestClass1[] obj = cache.get(1); //This will fail with
>> ClassCastException.
>>
>> assertEquals(TestClass1[].class, obj.getClass());
>> }
>> }
>> ```
>>
>>> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
>> написал(а):
>>>
>>> Thanks, Ilya.
>>>
>>> Can you put more context on this?
>>> I don’t familiar with these issues.
>>>
>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com>
>> написал(а):
>>>>
>>>> Hello!
>>>>
>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>>>> Fields.
>>>>
>>>> Regards,
>>>> --
>>>> Ilya Kasnacheev
>>>>
>>>>
>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
>>>>
>>>>> Hello,
>>>>>
>>>>>> However, for internal platform and services implementations we should
>>>>> fix the root cause:
>>>>>> avoid extra deserialization->serialization pass completely.
>>>>>> This will also improve performance.
>>>>>
>>>>> Pavel, thanks for the feedback.
>>>>> If I understand correctly, your suggestion is to know data size at the
>>>>> start of reading service parameters.
>>>>> Is it correct?
>>>>>
>>>>> Right now, when the service method invoked we pass an array of
>> parameters
>>>>> through platform reader/writer machinery.
>>>>> On java side parameters read one by one and we don’t know the size of
>> the
>>>>> data on the read start.
>>>>>
>>>>> AFAICU, this will require x2 memory on the .Net or thin client-side.
>>>>>
>>>>>
>>>>>
>> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
>>>>>
>>>>>
>>>>>> if we are to break compatibility, I would like to see it done for some
>>>>> really common pain point.
>>>>>
>>>>> Ilya, can you, please, provide a list of common issues with Ignite that
>>>>> can be resolved
>>>>> only with compatibility breakage?
>>>>>
>>>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
>>>>> написал(а):
>>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> If we really decide to break some compatibility then Array to
>>>>> BinaryObject
>>>>>> serialization will be very, very low on my personal list.
>>>>>>
>>>>>> I just don't see how this issue is relevant. I have been reading and
>>>>>> answering user list for a few years now, and I don't remember a single
>>>>>> question about storing ConcreteType[] as value and complaining about
>> type
>>>>>> information loss.
>>>>>>
>>>>>> If you have a good scenario how do we keep persistent store binary
>>>>>> compatibility here, without adding a lot of new code and still
>> checking
>>>>> for
>>>>>> both old and new approaches - you can go forward for sure.
>>>>>>
>>>>>> However, it does seem questionable where we have a new wrapper class
>>>>>> specifically for top level arrays. You can have this wrapper in your
>> own
>>>>>> client code and it should work OK.
>>>>>>
>>>>>> Bottom line, if we are to break compatibility, I would like to see it
>>>>> done
>>>>>> for some really common pain point.
>>>>>>
>>>>>> Regards,
>>>>>> --
>>>>>> Ilya Kasnacheev
>>>>>>
>>>>>>
>>>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
>>>>>>
>>>>>>> Hello, Ilya.
>>>>>>>
>>>>>>> Thanks for the feedback!
>>>>>>>
>>>>>>>> For me it sounds like something we would like to do in 3.0
>>>>>>>
>>>>>>> Ignite 3 is a very long way to go, so I prefer to target this fix in
>>>>>>> Ignite 2.x.
>>>>>>>
>>>>>>>> I think making it default "true" is a breaking change and is not
>>>>>>> possible in a minor release
>>>>>>>
>>>>>>> Yes, you are correct it is a breaking change.
>>>>>>> It seems for me, we all agreed that breaking changes are possible in
>>>>> minor
>>>>>>> releases.
>>>>>>>
>>>>>>> Anyway, if we will decide do not to enable this feature by default
>> it’s
>>>>> OK
>>>>>>> for me.
>>>>>>> We still can implement it and improve the binary SerDe mechanism.
>>>>>>>
>>>>>>>
>>>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
>> ilya.kasnacheev@gmail.com>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> Hello!
>>>>>>>>
>>>>>>>> For me it sounds like something we would like to do in 3.0 (if
>> indeed
>>>>> it
>>>>>>>> will have arrays as possible value (or key) type), but doing it in
>> 2.x
>>>>>>>> raises concerns whether it has enough time left to stabilize.
>>>>>>>>
>>>>>>>> Also, I think making it default "true" is a breaking change and is
>> not
>>>>>>>> possible in a minor release, case in point,
>>>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
>>>>> less
>>>>>>>> destructive.
>>>>>>>>
>>>>>>>> Of course I would also like to hear what other community members
>> think.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> --
>>>>>>>> Ilya Kasnacheev
>>>>>>>>
>>>>>>>>
>>>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
>>>>>>>>
>>>>>>>>> Igniters,
>>>>>>>>>
>>>>>>>>> Want to clarify my proposal about new array store format.
>>>>>>>>> I think we should store array in special binary wrapper that will
>> keep
>>>>>>>>> original component type
>>>>>>>>>
>>>>>>>>> ```
>>>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
>>>>>>> Externalizable {
>>>>>>>>> /** Type ID. */
>>>>>>>>> private int compTypeId;
>>>>>>>>>
>>>>>>>>> /** Raw data. */
>>>>>>>>> private String compClsName;
>>>>>>>>>
>>>>>>>>> /** Value. */
>>>>>>>>> private Object[] arr;
>>>>>>>>>
>>>>>>>>> // Further implementation.
>>>>>>>>> }
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <nizhikov.dev@gmail.com
>>>
>>>>>>>>> написал(а):
>>>>>>>>>>
>>>>>>>>>> Hello, Igniters.
>>>>>>>>>>
>>>>>>>>>> Currently, binary marshaller works as follows(Say, we have a class
>>>>>>>>> `User` then):
>>>>>>>>>>
>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>>>>>>>>
>>>>>>>>>> This means, that we lose array component type information during
>>>>> binary
>>>>>>>>> serialization.
>>>>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
>>>>>>>>> development.
>>>>>>>>>>
>>>>>>>>>> This lead to the following disadvantages:
>>>>>>>>>>
>>>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
>>>>> and
>>>>>>>>> hacks to deal with custom user array and still has many issues [1]
>>>>>>>>>>
>>>>>>>>>> I propose to make breaking changes and fix the custom user array
>> SeDe
>>>>>>> as
>>>>>>>>> follows:
>>>>>>>>>>
>>>>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
>>>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>>>>>>>>
>>>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>>>>>>>>
>>>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>>>>>>>>> correct SerDe of arrays. The default value is false to keep
>> backward
>>>>>>>>> compatibility in the next Ignite release(2.11).
>>>>>>>>>>
>>>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
>> Ignite
>>>>>>>>> releases (2.12).
>>>>>>>>>>
>>>>>>>>>> WDYT?
>>>>>>>>>>
>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>
>>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
Yes, it does not look pretty, I agree. But I'm saying that I've not
encountered this issue on the user list before, and it can probably be
easily countered by storing some one-field ArrayWrapper object in the cache
as value.
Maybe we should just automate that, e.g., whenever user stores Type[],
always store one-field ArrayWrapper object, and automatically unwrap it on
get()
This way we may not even need any changes to storage format, if binary
marshaller does not mind.
Regards,
Regards,
--
Ilya Kasnacheev
ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <ni...@apache.org>:
> Igniters.
>
> Just to clarify the issue:
>
> ```
> public class BinaryObjectTest extends GridCommonAbstractTest {
> /** */
> @Test
> public void testArray() throws Exception {
> Ignite ign = startGrid();
>
> IgniteCache<Integer, TestClass1[]> cache =
> ign.createCache("my-cache");
>
> cache.put(1, new TestClass1[] {new TestClass1(), new
> TestClass1()});
> TestClass1[] obj = cache.get(1); //This will fail with
> ClassCastException.
>
> assertEquals(TestClass1[].class, obj.getClass());
> }
> }
> ```
>
> > 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com>
> написал(а):
> >
> > Thanks, Ilya.
> >
> > Can you put more context on this?
> > I don’t familiar with these issues.
> >
> >> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >>
> >> Hello!
> >>
> >> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> >> Fields.
> >>
> >> Regards,
> >> --
> >> Ilya Kasnacheev
> >>
> >>
> >> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
> >>
> >>> Hello,
> >>>
> >>>> However, for internal platform and services implementations we should
> >>> fix the root cause:
> >>>> avoid extra deserialization->serialization pass completely.
> >>>> This will also improve performance.
> >>>
> >>> Pavel, thanks for the feedback.
> >>> If I understand correctly, your suggestion is to know data size at the
> >>> start of reading service parameters.
> >>> Is it correct?
> >>>
> >>> Right now, when the service method invoked we pass an array of
> parameters
> >>> through platform reader/writer machinery.
> >>> On java side parameters read one by one and we don’t know the size of
> the
> >>> data on the read start.
> >>>
> >>> AFAICU, this will require x2 memory on the .Net or thin client-side.
> >>>
> >>>
> >>>
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> >>>
> >>>
> >>>> if we are to break compatibility, I would like to see it done for some
> >>> really common pain point.
> >>>
> >>> Ilya, can you, please, provide a list of common issues with Ignite that
> >>> can be resolved
> >>> only with compatibility breakage?
> >>>
> >>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
> >>> написал(а):
> >>>>
> >>>> Hello!
> >>>>
> >>>> If we really decide to break some compatibility then Array to
> >>> BinaryObject
> >>>> serialization will be very, very low on my personal list.
> >>>>
> >>>> I just don't see how this issue is relevant. I have been reading and
> >>>> answering user list for a few years now, and I don't remember a single
> >>>> question about storing ConcreteType[] as value and complaining about
> type
> >>>> information loss.
> >>>>
> >>>> If you have a good scenario how do we keep persistent store binary
> >>>> compatibility here, without adding a lot of new code and still
> checking
> >>> for
> >>>> both old and new approaches - you can go forward for sure.
> >>>>
> >>>> However, it does seem questionable where we have a new wrapper class
> >>>> specifically for top level arrays. You can have this wrapper in your
> own
> >>>> client code and it should work OK.
> >>>>
> >>>> Bottom line, if we are to break compatibility, I would like to see it
> >>> done
> >>>> for some really common pain point.
> >>>>
> >>>> Regards,
> >>>> --
> >>>> Ilya Kasnacheev
> >>>>
> >>>>
> >>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
> >>>>
> >>>>> Hello, Ilya.
> >>>>>
> >>>>> Thanks for the feedback!
> >>>>>
> >>>>>> For me it sounds like something we would like to do in 3.0
> >>>>>
> >>>>> Ignite 3 is a very long way to go, so I prefer to target this fix in
> >>>>> Ignite 2.x.
> >>>>>
> >>>>>> I think making it default "true" is a breaking change and is not
> >>>>> possible in a minor release
> >>>>>
> >>>>> Yes, you are correct it is a breaking change.
> >>>>> It seems for me, we all agreed that breaking changes are possible in
> >>> minor
> >>>>> releases.
> >>>>>
> >>>>> Anyway, if we will decide do not to enable this feature by default
> it’s
> >>> OK
> >>>>> for me.
> >>>>> We still can implement it and improve the binary SerDe mechanism.
> >>>>>
> >>>>>
> >>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com>
> >>>>> написал(а):
> >>>>>>
> >>>>>> Hello!
> >>>>>>
> >>>>>> For me it sounds like something we would like to do in 3.0 (if
> indeed
> >>> it
> >>>>>> will have arrays as possible value (or key) type), but doing it in
> 2.x
> >>>>>> raises concerns whether it has enough time left to stabilize.
> >>>>>>
> >>>>>> Also, I think making it default "true" is a breaking change and is
> not
> >>>>>> possible in a minor release, case in point,
> >>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
> >>> less
> >>>>>> destructive.
> >>>>>>
> >>>>>> Of course I would also like to hear what other community members
> think.
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Ilya Kasnacheev
> >>>>>>
> >>>>>>
> >>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> >>>>>>
> >>>>>>> Igniters,
> >>>>>>>
> >>>>>>> Want to clarify my proposal about new array store format.
> >>>>>>> I think we should store array in special binary wrapper that will
> keep
> >>>>>>> original component type
> >>>>>>>
> >>>>>>> ```
> >>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> >>>>> Externalizable {
> >>>>>>> /** Type ID. */
> >>>>>>> private int compTypeId;
> >>>>>>>
> >>>>>>> /** Raw data. */
> >>>>>>> private String compClsName;
> >>>>>>>
> >>>>>>> /** Value. */
> >>>>>>> private Object[] arr;
> >>>>>>>
> >>>>>>> // Further implementation.
> >>>>>>> }
> >>>>>>> ```
> >>>>>>>
> >>>>>>>
> >>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <nizhikov.dev@gmail.com
> >
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> Hello, Igniters.
> >>>>>>>>
> >>>>>>>> Currently, binary marshaller works as follows(Say, we have a class
> >>>>>>> `User` then):
> >>>>>>>>
> >>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
> >>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>>>>>>
> >>>>>>>> This means, that we lose array component type information during
> >>> binary
> >>>>>>> serialization.
> >>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
> >>>>>>> development.
> >>>>>>>>
> >>>>>>>> This lead to the following disadvantages:
> >>>>>>>>
> >>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
> >>> and
> >>>>>>> hacks to deal with custom user array and still has many issues [1]
> >>>>>>>>
> >>>>>>>> I propose to make breaking changes and fix the custom user array
> SeDe
> >>>>> as
> >>>>>>> follows:
> >>>>>>>>
> >>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
> >>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>>>>>>
> >>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>>>>>>
> >>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >>>>>>> correct SerDe of arrays. The default value is false to keep
> backward
> >>>>>>> compatibility in the next Ignite release(2.11).
> >>>>>>>>
> >>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> Ignite
> >>>>>>> releases (2.12).
> >>>>>>>>
> >>>>>>>> WDYT?
> >>>>>>>>
> >>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Igniters.
Just to clarify the issue:
```
public class BinaryObjectTest extends GridCommonAbstractTest {
/** */
@Test
public void testArray() throws Exception {
Ignite ign = startGrid();
IgniteCache<Integer, TestClass1[]> cache = ign.createCache("my-cache");
cache.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
TestClass1[] obj = cache.get(1); //This will fail with ClassCastException.
assertEquals(TestClass1[].class, obj.getClass());
}
}
```
> 19 мая 2021 г., в 13:04, Nikolay Izhikov <ni...@gmail.com> написал(а):
>
> Thanks, Ilya.
>
> Can you put more context on this?
> I don’t familiar with these issues.
>
>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com> написал(а):
>>
>> Hello!
>>
>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>> Fields.
>>
>> Regards,
>> --
>> Ilya Kasnacheev
>>
>>
>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
>>
>>> Hello,
>>>
>>>> However, for internal platform and services implementations we should
>>> fix the root cause:
>>>> avoid extra deserialization->serialization pass completely.
>>>> This will also improve performance.
>>>
>>> Pavel, thanks for the feedback.
>>> If I understand correctly, your suggestion is to know data size at the
>>> start of reading service parameters.
>>> Is it correct?
>>>
>>> Right now, when the service method invoked we pass an array of parameters
>>> through platform reader/writer machinery.
>>> On java side parameters read one by one and we don’t know the size of the
>>> data on the read start.
>>>
>>> AFAICU, this will require x2 memory on the .Net or thin client-side.
>>>
>>>
>>> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
>>>
>>>
>>>> if we are to break compatibility, I would like to see it done for some
>>> really common pain point.
>>>
>>> Ilya, can you, please, provide a list of common issues with Ignite that
>>> can be resolved
>>> only with compatibility breakage?
>>>
>>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
>>> написал(а):
>>>>
>>>> Hello!
>>>>
>>>> If we really decide to break some compatibility then Array to
>>> BinaryObject
>>>> serialization will be very, very low on my personal list.
>>>>
>>>> I just don't see how this issue is relevant. I have been reading and
>>>> answering user list for a few years now, and I don't remember a single
>>>> question about storing ConcreteType[] as value and complaining about type
>>>> information loss.
>>>>
>>>> If you have a good scenario how do we keep persistent store binary
>>>> compatibility here, without adding a lot of new code and still checking
>>> for
>>>> both old and new approaches - you can go forward for sure.
>>>>
>>>> However, it does seem questionable where we have a new wrapper class
>>>> specifically for top level arrays. You can have this wrapper in your own
>>>> client code and it should work OK.
>>>>
>>>> Bottom line, if we are to break compatibility, I would like to see it
>>> done
>>>> for some really common pain point.
>>>>
>>>> Regards,
>>>> --
>>>> Ilya Kasnacheev
>>>>
>>>>
>>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
>>>>
>>>>> Hello, Ilya.
>>>>>
>>>>> Thanks for the feedback!
>>>>>
>>>>>> For me it sounds like something we would like to do in 3.0
>>>>>
>>>>> Ignite 3 is a very long way to go, so I prefer to target this fix in
>>>>> Ignite 2.x.
>>>>>
>>>>>> I think making it default "true" is a breaking change and is not
>>>>> possible in a minor release
>>>>>
>>>>> Yes, you are correct it is a breaking change.
>>>>> It seems for me, we all agreed that breaking changes are possible in
>>> minor
>>>>> releases.
>>>>>
>>>>> Anyway, if we will decide do not to enable this feature by default it’s
>>> OK
>>>>> for me.
>>>>> We still can implement it and improve the binary SerDe mechanism.
>>>>>
>>>>>
>>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
>>>>> написал(а):
>>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> For me it sounds like something we would like to do in 3.0 (if indeed
>>> it
>>>>>> will have arrays as possible value (or key) type), but doing it in 2.x
>>>>>> raises concerns whether it has enough time left to stabilize.
>>>>>>
>>>>>> Also, I think making it default "true" is a breaking change and is not
>>>>>> possible in a minor release, case in point,
>>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
>>> less
>>>>>> destructive.
>>>>>>
>>>>>> Of course I would also like to hear what other community members think.
>>>>>>
>>>>>> Regards,
>>>>>> --
>>>>>> Ilya Kasnacheev
>>>>>>
>>>>>>
>>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
>>>>>>
>>>>>>> Igniters,
>>>>>>>
>>>>>>> Want to clarify my proposal about new array store format.
>>>>>>> I think we should store array in special binary wrapper that will keep
>>>>>>> original component type
>>>>>>>
>>>>>>> ```
>>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
>>>>> Externalizable {
>>>>>>> /** Type ID. */
>>>>>>> private int compTypeId;
>>>>>>>
>>>>>>> /** Raw data. */
>>>>>>> private String compClsName;
>>>>>>>
>>>>>>> /** Value. */
>>>>>>> private Object[] arr;
>>>>>>>
>>>>>>> // Further implementation.
>>>>>>> }
>>>>>>> ```
>>>>>>>
>>>>>>>
>>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> Hello, Igniters.
>>>>>>>>
>>>>>>>> Currently, binary marshaller works as follows(Say, we have a class
>>>>>>> `User` then):
>>>>>>>>
>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
>>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>>>>>>
>>>>>>>> This means, that we lose array component type information during
>>> binary
>>>>>>> serialization.
>>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
>>>>>>> development.
>>>>>>>>
>>>>>>>> This lead to the following disadvantages:
>>>>>>>>
>>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
>>> and
>>>>>>> hacks to deal with custom user array and still has many issues [1]
>>>>>>>>
>>>>>>>> I propose to make breaking changes and fix the custom user array SeDe
>>>>> as
>>>>>>> follows:
>>>>>>>>
>>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
>>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>>>>>>
>>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>>>>>>
>>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>>>>>>> correct SerDe of arrays. The default value is false to keep backward
>>>>>>> compatibility in the next Ignite release(2.11).
>>>>>>>>
>>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
>>>>>>> releases (2.12).
>>>>>>>>
>>>>>>>> WDYT?
>>>>>>>>
>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Thanks, Ilya.
Can you put more context on this?
I don’t familiar with these issues.
> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <il...@gmail.com> написал(а):
>
> Hello!
>
> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> Fields.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
>
>> Hello,
>>
>>> However, for internal platform and services implementations we should
>> fix the root cause:
>>> avoid extra deserialization->serialization pass completely.
>>> This will also improve performance.
>>
>> Pavel, thanks for the feedback.
>> If I understand correctly, your suggestion is to know data size at the
>> start of reading service parameters.
>> Is it correct?
>>
>> Right now, when the service method invoked we pass an array of parameters
>> through platform reader/writer machinery.
>> On java side parameters read one by one and we don’t know the size of the
>> data on the read start.
>>
>> AFAICU, this will require x2 memory on the .Net or thin client-side.
>>
>>
>> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
>>
>>
>>> if we are to break compatibility, I would like to see it done for some
>> really common pain point.
>>
>> Ilya, can you, please, provide a list of common issues with Ignite that
>> can be resolved
>> only with compatibility breakage?
>>
>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
>> написал(а):
>>>
>>> Hello!
>>>
>>> If we really decide to break some compatibility then Array to
>> BinaryObject
>>> serialization will be very, very low on my personal list.
>>>
>>> I just don't see how this issue is relevant. I have been reading and
>>> answering user list for a few years now, and I don't remember a single
>>> question about storing ConcreteType[] as value and complaining about type
>>> information loss.
>>>
>>> If you have a good scenario how do we keep persistent store binary
>>> compatibility here, without adding a lot of new code and still checking
>> for
>>> both old and new approaches - you can go forward for sure.
>>>
>>> However, it does seem questionable where we have a new wrapper class
>>> specifically for top level arrays. You can have this wrapper in your own
>>> client code and it should work OK.
>>>
>>> Bottom line, if we are to break compatibility, I would like to see it
>> done
>>> for some really common pain point.
>>>
>>> Regards,
>>> --
>>> Ilya Kasnacheev
>>>
>>>
>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
>>>
>>>> Hello, Ilya.
>>>>
>>>> Thanks for the feedback!
>>>>
>>>>> For me it sounds like something we would like to do in 3.0
>>>>
>>>> Ignite 3 is a very long way to go, so I prefer to target this fix in
>>>> Ignite 2.x.
>>>>
>>>>> I think making it default "true" is a breaking change and is not
>>>> possible in a minor release
>>>>
>>>> Yes, you are correct it is a breaking change.
>>>> It seems for me, we all agreed that breaking changes are possible in
>> minor
>>>> releases.
>>>>
>>>> Anyway, if we will decide do not to enable this feature by default it’s
>> OK
>>>> for me.
>>>> We still can implement it and improve the binary SerDe mechanism.
>>>>
>>>>
>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
>>>> написал(а):
>>>>>
>>>>> Hello!
>>>>>
>>>>> For me it sounds like something we would like to do in 3.0 (if indeed
>> it
>>>>> will have arrays as possible value (or key) type), but doing it in 2.x
>>>>> raises concerns whether it has enough time left to stabilize.
>>>>>
>>>>> Also, I think making it default "true" is a breaking change and is not
>>>>> possible in a minor release, case in point,
>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
>> less
>>>>> destructive.
>>>>>
>>>>> Of course I would also like to hear what other community members think.
>>>>>
>>>>> Regards,
>>>>> --
>>>>> Ilya Kasnacheev
>>>>>
>>>>>
>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
>>>>>
>>>>>> Igniters,
>>>>>>
>>>>>> Want to clarify my proposal about new array store format.
>>>>>> I think we should store array in special binary wrapper that will keep
>>>>>> original component type
>>>>>>
>>>>>> ```
>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
>>>> Externalizable {
>>>>>> /** Type ID. */
>>>>>> private int compTypeId;
>>>>>>
>>>>>> /** Raw data. */
>>>>>> private String compClsName;
>>>>>>
>>>>>> /** Value. */
>>>>>> private Object[] arr;
>>>>>>
>>>>>> // Further implementation.
>>>>>> }
>>>>>> ```
>>>>>>
>>>>>>
>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
>>>>>> написал(а):
>>>>>>>
>>>>>>> Hello, Igniters.
>>>>>>>
>>>>>>> Currently, binary marshaller works as follows(Say, we have a class
>>>>>> `User` then):
>>>>>>>
>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>>>>>
>>>>>>> This means, that we lose array component type information during
>> binary
>>>>>> serialization.
>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
>>>>>> development.
>>>>>>>
>>>>>>> This lead to the following disadvantages:
>>>>>>>
>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
>> and
>>>>>> hacks to deal with custom user array and still has many issues [1]
>>>>>>>
>>>>>>> I propose to make breaking changes and fix the custom user array SeDe
>>>> as
>>>>>> follows:
>>>>>>>
>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>>>>>
>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>>>>>
>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>>>>>> correct SerDe of arrays. The default value is false to keep backward
>>>>>> compatibility in the next Ignite release(2.11).
>>>>>>>
>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
>>>>>> releases (2.12).
>>>>>>>
>>>>>>> WDYT?
>>>>>>>
>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
Fields.
Regards,
--
Ilya Kasnacheev
ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <ni...@apache.org>:
> Hello,
>
> > However, for internal platform and services implementations we should
> fix the root cause:
> > avoid extra deserialization->serialization pass completely.
> > This will also improve performance.
>
> Pavel, thanks for the feedback.
> If I understand correctly, your suggestion is to know data size at the
> start of reading service parameters.
> Is it correct?
>
> Right now, when the service method invoked we pass an array of parameters
> through platform reader/writer machinery.
> On java side parameters read one by one and we don’t know the size of the
> data on the read start.
>
> AFAICU, this will require x2 memory on the .Net or thin client-side.
>
>
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
>
>
> > if we are to break compatibility, I would like to see it done for some
> really common pain point.
>
> Ilya, can you, please, provide a list of common issues with Ignite that
> can be resolved
> only with compatibility breakage?
>
> > 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > If we really decide to break some compatibility then Array to
> BinaryObject
> > serialization will be very, very low on my personal list.
> >
> > I just don't see how this issue is relevant. I have been reading and
> > answering user list for a few years now, and I don't remember a single
> > question about storing ConcreteType[] as value and complaining about type
> > information loss.
> >
> > If you have a good scenario how do we keep persistent store binary
> > compatibility here, without adding a lot of new code and still checking
> for
> > both old and new approaches - you can go forward for sure.
> >
> > However, it does seem questionable where we have a new wrapper class
> > specifically for top level arrays. You can have this wrapper in your own
> > client code and it should work OK.
> >
> > Bottom line, if we are to break compatibility, I would like to see it
> done
> > for some really common pain point.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
> >
> >> Hello, Ilya.
> >>
> >> Thanks for the feedback!
> >>
> >>> For me it sounds like something we would like to do in 3.0
> >>
> >> Ignite 3 is a very long way to go, so I prefer to target this fix in
> >> Ignite 2.x.
> >>
> >>> I think making it default "true" is a breaking change and is not
> >> possible in a minor release
> >>
> >> Yes, you are correct it is a breaking change.
> >> It seems for me, we all agreed that breaking changes are possible in
> minor
> >> releases.
> >>
> >> Anyway, if we will decide do not to enable this feature by default it’s
> OK
> >> for me.
> >> We still can implement it and improve the binary SerDe mechanism.
> >>
> >>
> >>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
> >> написал(а):
> >>>
> >>> Hello!
> >>>
> >>> For me it sounds like something we would like to do in 3.0 (if indeed
> it
> >>> will have arrays as possible value (or key) type), but doing it in 2.x
> >>> raises concerns whether it has enough time left to stabilize.
> >>>
> >>> Also, I think making it default "true" is a breaking change and is not
> >>> possible in a minor release, case in point,
> >>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
> less
> >>> destructive.
> >>>
> >>> Of course I would also like to hear what other community members think.
> >>>
> >>> Regards,
> >>> --
> >>> Ilya Kasnacheev
> >>>
> >>>
> >>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> >>>
> >>>> Igniters,
> >>>>
> >>>> Want to clarify my proposal about new array store format.
> >>>> I think we should store array in special binary wrapper that will keep
> >>>> original component type
> >>>>
> >>>> ```
> >>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> >> Externalizable {
> >>>> /** Type ID. */
> >>>> private int compTypeId;
> >>>>
> >>>> /** Raw data. */
> >>>> private String compClsName;
> >>>>
> >>>> /** Value. */
> >>>> private Object[] arr;
> >>>>
> >>>> // Further implementation.
> >>>> }
> >>>> ```
> >>>>
> >>>>
> >>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> >>>> написал(а):
> >>>>>
> >>>>> Hello, Igniters.
> >>>>>
> >>>>> Currently, binary marshaller works as follows(Say, we have a class
> >>>> `User` then):
> >>>>>
> >>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>> IgniteBinary#toBinary(User[])` -> Object[]
> >>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>>>
> >>>>> This means, that we lose array component type information during
> binary
> >>>> serialization.
> >>>>> AFAIK, it’s a design choice made during binary infrastructure
> >>>> development.
> >>>>>
> >>>>> This lead to the following disadvantages:
> >>>>>
> >>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
> and
> >>>> hacks to deal with custom user array and still has many issues [1]
> >>>>>
> >>>>> I propose to make breaking changes and fix the custom user array SeDe
> >> as
> >>>> follows:
> >>>>>
> >>>>> 1. Implement binary serialization that correctly Ser and Deser
> >>>> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>>>
> >>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>>>
> >>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >>>> correct SerDe of arrays. The default value is false to keep backward
> >>>> compatibility in the next Ignite release(2.11).
> >>>>>
> >>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> >>>> releases (2.12).
> >>>>>
> >>>>> WDYT?
> >>>>>
> >>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>>>
> >>>>
> >>
> >>
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Hello,
> However, for internal platform and services implementations we should fix the root cause:
> avoid extra deserialization->serialization pass completely.
> This will also improve performance.
Pavel, thanks for the feedback.
If I understand correctly, your suggestion is to know data size at the start of reading service parameters.
Is it correct?
Right now, when the service method invoked we pass an array of parameters through platform reader/writer machinery.
On java side parameters read one by one and we don’t know the size of the data on the read start.
AFAICU, this will require x2 memory on the .Net or thin client-side.
https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> if we are to break compatibility, I would like to see it done for some really common pain point.
Ilya, can you, please, provide a list of common issues with Ignite that can be resolved
only with compatibility breakage?
> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <il...@gmail.com> написал(а):
>
> Hello!
>
> If we really decide to break some compatibility then Array to BinaryObject
> serialization will be very, very low on my personal list.
>
> I just don't see how this issue is relevant. I have been reading and
> answering user list for a few years now, and I don't remember a single
> question about storing ConcreteType[] as value and complaining about type
> information loss.
>
> If you have a good scenario how do we keep persistent store binary
> compatibility here, without adding a lot of new code and still checking for
> both old and new approaches - you can go forward for sure.
>
> However, it does seem questionable where we have a new wrapper class
> specifically for top level arrays. You can have this wrapper in your own
> client code and it should work OK.
>
> Bottom line, if we are to break compatibility, I would like to see it done
> for some really common pain point.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
>
>> Hello, Ilya.
>>
>> Thanks for the feedback!
>>
>>> For me it sounds like something we would like to do in 3.0
>>
>> Ignite 3 is a very long way to go, so I prefer to target this fix in
>> Ignite 2.x.
>>
>>> I think making it default "true" is a breaking change and is not
>> possible in a minor release
>>
>> Yes, you are correct it is a breaking change.
>> It seems for me, we all agreed that breaking changes are possible in minor
>> releases.
>>
>> Anyway, if we will decide do not to enable this feature by default it’s OK
>> for me.
>> We still can implement it and improve the binary SerDe mechanism.
>>
>>
>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
>> написал(а):
>>>
>>> Hello!
>>>
>>> For me it sounds like something we would like to do in 3.0 (if indeed it
>>> will have arrays as possible value (or key) type), but doing it in 2.x
>>> raises concerns whether it has enough time left to stabilize.
>>>
>>> Also, I think making it default "true" is a breaking change and is not
>>> possible in a minor release, case in point,
>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
>>> destructive.
>>>
>>> Of course I would also like to hear what other community members think.
>>>
>>> Regards,
>>> --
>>> Ilya Kasnacheev
>>>
>>>
>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
>>>
>>>> Igniters,
>>>>
>>>> Want to clarify my proposal about new array store format.
>>>> I think we should store array in special binary wrapper that will keep
>>>> original component type
>>>>
>>>> ```
>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
>> Externalizable {
>>>> /** Type ID. */
>>>> private int compTypeId;
>>>>
>>>> /** Raw data. */
>>>> private String compClsName;
>>>>
>>>> /** Value. */
>>>> private Object[] arr;
>>>>
>>>> // Further implementation.
>>>> }
>>>> ```
>>>>
>>>>
>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
>>>> написал(а):
>>>>>
>>>>> Hello, Igniters.
>>>>>
>>>>> Currently, binary marshaller works as follows(Say, we have a class
>>>> `User` then):
>>>>>
>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>> IgniteBinary#toBinary(User[])` -> Object[]
>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>>>
>>>>> This means, that we lose array component type information during binary
>>>> serialization.
>>>>> AFAIK, it’s a design choice made during binary infrastructure
>>>> development.
>>>>>
>>>>> This lead to the following disadvantages:
>>>>>
>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
>>>> hacks to deal with custom user array and still has many issues [1]
>>>>>
>>>>> I propose to make breaking changes and fix the custom user array SeDe
>> as
>>>> follows:
>>>>>
>>>>> 1. Implement binary serialization that correctly Ser and Deser
>>>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>>>
>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>>>
>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>>>> correct SerDe of arrays. The default value is false to keep backward
>>>> compatibility in the next Ignite release(2.11).
>>>>>
>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
>>>> releases (2.12).
>>>>>
>>>>> WDYT?
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>>>
>>>>
>>
>>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
If we really decide to break some compatibility then Array to BinaryObject
serialization will be very, very low on my personal list.
I just don't see how this issue is relevant. I have been reading and
answering user list for a few years now, and I don't remember a single
question about storing ConcreteType[] as value and complaining about type
information loss.
If you have a good scenario how do we keep persistent store binary
compatibility here, without adding a lot of new code and still checking for
both old and new approaches - you can go forward for sure.
However, it does seem questionable where we have a new wrapper class
specifically for top level arrays. You can have this wrapper in your own
client code and it should work OK.
Bottom line, if we are to break compatibility, I would like to see it done
for some really common pain point.
Regards,
--
Ilya Kasnacheev
пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <ni...@apache.org>:
> Hello, Ilya.
>
> Thanks for the feedback!
>
> > For me it sounds like something we would like to do in 3.0
>
> Ignite 3 is a very long way to go, so I prefer to target this fix in
> Ignite 2.x.
>
> > I think making it default "true" is a breaking change and is not
> possible in a minor release
>
> Yes, you are correct it is a breaking change.
> It seems for me, we all agreed that breaking changes are possible in minor
> releases.
>
> Anyway, if we will decide do not to enable this feature by default it’s OK
> for me.
> We still can implement it and improve the binary SerDe mechanism.
>
>
> > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > For me it sounds like something we would like to do in 3.0 (if indeed it
> > will have arrays as possible value (or key) type), but doing it in 2.x
> > raises concerns whether it has enough time left to stabilize.
> >
> > Also, I think making it default "true" is a breaking change and is not
> > possible in a minor release, case in point,
> > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
> > destructive.
> >
> > Of course I would also like to hear what other community members think.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> >
> >> Igniters,
> >>
> >> Want to clarify my proposal about new array store format.
> >> I think we should store array in special binary wrapper that will keep
> >> original component type
> >>
> >> ```
> >> public class BinaryArrayWrapper implements BinaryObjectEx,
> Externalizable {
> >> /** Type ID. */
> >> private int compTypeId;
> >>
> >> /** Raw data. */
> >> private String compClsName;
> >>
> >> /** Value. */
> >> private Object[] arr;
> >>
> >> // Further implementation.
> >> }
> >> ```
> >>
> >>
> >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> >> написал(а):
> >>>
> >>> Hello, Igniters.
> >>>
> >>> Currently, binary marshaller works as follows(Say, we have a class
> >> `User` then):
> >>>
> >>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>> IgniteBinary#toBinary(User[])` -> Object[]
> >>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>
> >>> This means, that we lose array component type information during binary
> >> serialization.
> >>> AFAIK, it’s a design choice made during binary infrastructure
> >> development.
> >>>
> >>> This lead to the following disadvantages:
> >>>
> >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
> >> hacks to deal with custom user array and still has many issues [1]
> >>>
> >>> I propose to make breaking changes and fix the custom user array SeDe
> as
> >> follows:
> >>>
> >>> 1. Implement binary serialization that correctly Ser and Deser
> >> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>
> >>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>
> >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >> correct SerDe of arrays. The default value is false to keep backward
> >> compatibility in the next Ignite release(2.11).
> >>>
> >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> >> releases (2.12).
> >>>
> >>> WDYT?
> >>>
> >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>
> >>
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Pavel Tupitsyn <pt...@apache.org>.
Nikolay,
I agree that our binary array handling has some limitations - Ignite loses
array element type in many cases (cache.Put -> cache.Get, Binary Mode, etc).
However, for internal platform and services implementations we should fix
the root cause:
avoid extra deserialization->serialization pass completely.
This will also improve performance.
Thanks,
Pavel
On Sat, May 1, 2021 at 9:20 PM Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> Hi Ivan,
>
> Thanks for chiming in. My comments are below.
>
> -Val
>
> On Sat, May 1, 2021 at 12:20 AM Ivan Daschinsky <iv...@gmail.com>
> wrote:
>
> > Hi!
> > First of all, when array is serialized, marshaller actually DO
> > PRESERVE type of element (seel
> > org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray and
> > org.apache.ignite.internal.binary.BinaryWriterExImpl#doWriteObjectArray).
> > AFAIK, the motivation of Nickolay proposal, is the fact, that during
> > call of PlatformService we do additional marshal/unmarshall step and
> > during this step we loose array type info
> > See
> org.apache.ignite.internal.binary.BinaryReaderExImpl#readObjectDetached
> > and org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray)
> >
> > In order to handle this problem, we can just add some wrapper that
> > contains element type info and use it in our INTERNAL API.
> > I just don't understand why we should change IgniteBinary API.
> >
>
> Makes sense to me. I would also avoid changing the public API.
>
>
> >
> > >> It was designed as a data storage format, and the fact
> > that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
> > something we're trying to fix in 3.0.
> >
> > Please, do not. There are many cases when this can really improve
> > performance. Reflection is always slower than low level api and many
> > users are happy with low level API.
> >
>
> Low-level APIs are not being removed. If anything, they are likely to
> become more low-level :) Either way, that's off-topic. I would recommend
> reviewing the related IEPs and starting a separate discussion if you have
> any questions or concerns.
>
>
> >
> > сб, 1 мая 2021 г. в 00:51, Valentin Kulichenko <
> > valentin.kulichenko@gmail.com>:
> > >
> > > Hi Nikolay,
> > >
> > > Is there a specific motivation behind your proposal? I do acknowledge
> > that
> > > the semantics of the toBinary method is a little weird, but my concern
> is
> > > that the way it works with arrays is just an example. Are you
> suggesting
> > > changing collections, primitives, and other "first citizen" data types
> as
> > > well? To me, that looks like a huge risky change without a clear value.
> > >
> > > I actually believe that binary format *should not* be used as a
> universal
> > > serde mechanism. It was designed as a data storage format, and the fact
> > > that Ignite uses it elsewhere is a fundamental design flaw. BTW, this
> is
> > > something we're trying to fix in 3.0.
> > >
> > > That said, I'm not necessarily against the change (especially if we do
> > not
> > > change the default behavior). But I would like to better understand its
> > > scope (e.g., arrays only or beyond?), as well as get some examples of
> use
> > > cases that would benefit from the change.
> > >
> > > -Val
> > >
> > >
> > > On Fri, Apr 30, 2021 at 7:35 AM Nikolay Izhikov <ni...@apache.org>
> > wrote:
> > >
> > > > Hello, Ilya.
> > > >
> > > > Thanks for the feedback!
> > > >
> > > > > For me it sounds like something we would like to do in 3.0
> > > >
> > > > Ignite 3 is a very long way to go, so I prefer to target this fix in
> > > > Ignite 2.x.
> > > >
> > > > > I think making it default "true" is a breaking change and is not
> > > > possible in a minor release
> > > >
> > > > Yes, you are correct it is a breaking change.
> > > > It seems for me, we all agreed that breaking changes are possible in
> > minor
> > > > releases.
> > > >
> > > > Anyway, if we will decide do not to enable this feature by default
> > it’s OK
> > > > for me.
> > > > We still can implement it and improve the binary SerDe mechanism.
> > > >
> > > >
> > > > > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com
> > >
> > > > написал(а):
> > > > >
> > > > > Hello!
> > > > >
> > > > > For me it sounds like something we would like to do in 3.0 (if
> > indeed it
> > > > > will have arrays as possible value (or key) type), but doing it in
> > 2.x
> > > > > raises concerns whether it has enough time left to stabilize.
> > > > >
> > > > > Also, I think making it default "true" is a breaking change and is
> > not
> > > > > possible in a minor release, case in point,
> > > > > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
> > less
> > > > > destructive.
> > > > >
> > > > > Of course I would also like to hear what other community members
> > think.
> > > > >
> > > > > Regards,
> > > > > --
> > > > > Ilya Kasnacheev
> > > > >
> > > > >
> > > > > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <nizhikov@apache.org
> >:
> > > > >
> > > > >> Igniters,
> > > > >>
> > > > >> Want to clarify my proposal about new array store format.
> > > > >> I think we should store array in special binary wrapper that will
> > keep
> > > > >> original component type
> > > > >>
> > > > >> ```
> > > > >> public class BinaryArrayWrapper implements BinaryObjectEx,
> > > > Externalizable {
> > > > >> /** Type ID. */
> > > > >> private int compTypeId;
> > > > >>
> > > > >> /** Raw data. */
> > > > >> private String compClsName;
> > > > >>
> > > > >> /** Value. */
> > > > >> private Object[] arr;
> > > > >>
> > > > >> // Further implementation.
> > > > >> }
> > > > >> ```
> > > > >>
> > > > >>
> > > > >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <
> nizhikov.dev@gmail.com>
> > > > >> написал(а):
> > > > >>>
> > > > >>> Hello, Igniters.
> > > > >>>
> > > > >>> Currently, binary marshaller works as follows(Say, we have a
> class
> > > > >> `User` then):
> > > > >>>
> > > > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > > > >>> IgniteBinary#toBinary(User[])` -> Object[]
> > > > >>> IgniteBinary#toBinary(Object[])` -> Object[]
> > > > >>>
> > > > >>> This means, that we lose array component type information during
> > binary
> > > > >> serialization.
> > > > >>> AFAIK, it’s a design choice made during binary infrastructure
> > > > >> development.
> > > > >>>
> > > > >>> This lead to the following disadvantages:
> > > > >>>
> > > > >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> > > > >>> 2. Ignite internals(service grid, .Net calls) contains many
> tweaks
> > and
> > > > >> hacks to deal with custom user array and still has many issues [1]
> > > > >>>
> > > > >>> I propose to make breaking changes and fix the custom user array
> > SeDe
> > > > as
> > > > >> follows:
> > > > >>>
> > > > >>> 1. Implement binary serialization that correctly Ser and
> Deser
> > > > >> array using some kind of the wrapper (BinaryArrayWrapper).
> > > > >>>
> > > > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > > > >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> > > > >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> > > > >>>
> > > > >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that
> > enables
> > > > >> correct SerDe of arrays. The default value is false to keep
> backward
> > > > >> compatibility in the next Ignite release(2.11).
> > > > >>>
> > > > >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> > Ignite
> > > > >> releases (2.12).
> > > > >>>
> > > > >>> WDYT?
> > > > >>>
> > > > >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> > > > >>
> > > > >>
> > > >
> > > >
> >
> >
> >
> > --
> > Sincerely yours, Ivan Daschinskiy
> >
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Valentin Kulichenko <va...@gmail.com>.
Hi Ivan,
Thanks for chiming in. My comments are below.
-Val
On Sat, May 1, 2021 at 12:20 AM Ivan Daschinsky <iv...@gmail.com> wrote:
> Hi!
> First of all, when array is serialized, marshaller actually DO
> PRESERVE type of element (seel
> org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray and
> org.apache.ignite.internal.binary.BinaryWriterExImpl#doWriteObjectArray).
> AFAIK, the motivation of Nickolay proposal, is the fact, that during
> call of PlatformService we do additional marshal/unmarshall step and
> during this step we loose array type info
> See org.apache.ignite.internal.binary.BinaryReaderExImpl#readObjectDetached
> and org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray)
>
> In order to handle this problem, we can just add some wrapper that
> contains element type info and use it in our INTERNAL API.
> I just don't understand why we should change IgniteBinary API.
>
Makes sense to me. I would also avoid changing the public API.
>
> >> It was designed as a data storage format, and the fact
> that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
> something we're trying to fix in 3.0.
>
> Please, do not. There are many cases when this can really improve
> performance. Reflection is always slower than low level api and many
> users are happy with low level API.
>
Low-level APIs are not being removed. If anything, they are likely to
become more low-level :) Either way, that's off-topic. I would recommend
reviewing the related IEPs and starting a separate discussion if you have
any questions or concerns.
>
> сб, 1 мая 2021 г. в 00:51, Valentin Kulichenko <
> valentin.kulichenko@gmail.com>:
> >
> > Hi Nikolay,
> >
> > Is there a specific motivation behind your proposal? I do acknowledge
> that
> > the semantics of the toBinary method is a little weird, but my concern is
> > that the way it works with arrays is just an example. Are you suggesting
> > changing collections, primitives, and other "first citizen" data types as
> > well? To me, that looks like a huge risky change without a clear value.
> >
> > I actually believe that binary format *should not* be used as a universal
> > serde mechanism. It was designed as a data storage format, and the fact
> > that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
> > something we're trying to fix in 3.0.
> >
> > That said, I'm not necessarily against the change (especially if we do
> not
> > change the default behavior). But I would like to better understand its
> > scope (e.g., arrays only or beyond?), as well as get some examples of use
> > cases that would benefit from the change.
> >
> > -Val
> >
> >
> > On Fri, Apr 30, 2021 at 7:35 AM Nikolay Izhikov <ni...@apache.org>
> wrote:
> >
> > > Hello, Ilya.
> > >
> > > Thanks for the feedback!
> > >
> > > > For me it sounds like something we would like to do in 3.0
> > >
> > > Ignite 3 is a very long way to go, so I prefer to target this fix in
> > > Ignite 2.x.
> > >
> > > > I think making it default "true" is a breaking change and is not
> > > possible in a minor release
> > >
> > > Yes, you are correct it is a breaking change.
> > > It seems for me, we all agreed that breaking changes are possible in
> minor
> > > releases.
> > >
> > > Anyway, if we will decide do not to enable this feature by default
> it’s OK
> > > for me.
> > > We still can implement it and improve the binary SerDe mechanism.
> > >
> > >
> > > > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <ilya.kasnacheev@gmail.com
> >
> > > написал(а):
> > > >
> > > > Hello!
> > > >
> > > > For me it sounds like something we would like to do in 3.0 (if
> indeed it
> > > > will have arrays as possible value (or key) type), but doing it in
> 2.x
> > > > raises concerns whether it has enough time left to stabilize.
> > > >
> > > > Also, I think making it default "true" is a breaking change and is
> not
> > > > possible in a minor release, case in point,
> > > > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
> less
> > > > destructive.
> > > >
> > > > Of course I would also like to hear what other community members
> think.
> > > >
> > > > Regards,
> > > > --
> > > > Ilya Kasnacheev
> > > >
> > > >
> > > > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> > > >
> > > >> Igniters,
> > > >>
> > > >> Want to clarify my proposal about new array store format.
> > > >> I think we should store array in special binary wrapper that will
> keep
> > > >> original component type
> > > >>
> > > >> ```
> > > >> public class BinaryArrayWrapper implements BinaryObjectEx,
> > > Externalizable {
> > > >> /** Type ID. */
> > > >> private int compTypeId;
> > > >>
> > > >> /** Raw data. */
> > > >> private String compClsName;
> > > >>
> > > >> /** Value. */
> > > >> private Object[] arr;
> > > >>
> > > >> // Further implementation.
> > > >> }
> > > >> ```
> > > >>
> > > >>
> > > >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> > > >> написал(а):
> > > >>>
> > > >>> Hello, Igniters.
> > > >>>
> > > >>> Currently, binary marshaller works as follows(Say, we have a class
> > > >> `User` then):
> > > >>>
> > > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > > >>> IgniteBinary#toBinary(User[])` -> Object[]
> > > >>> IgniteBinary#toBinary(Object[])` -> Object[]
> > > >>>
> > > >>> This means, that we lose array component type information during
> binary
> > > >> serialization.
> > > >>> AFAIK, it’s a design choice made during binary infrastructure
> > > >> development.
> > > >>>
> > > >>> This lead to the following disadvantages:
> > > >>>
> > > >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> > > >>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
> and
> > > >> hacks to deal with custom user array and still has many issues [1]
> > > >>>
> > > >>> I propose to make breaking changes and fix the custom user array
> SeDe
> > > as
> > > >> follows:
> > > >>>
> > > >>> 1. Implement binary serialization that correctly Ser and Deser
> > > >> array using some kind of the wrapper (BinaryArrayWrapper).
> > > >>>
> > > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > > >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> > > >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> > > >>>
> > > >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that
> enables
> > > >> correct SerDe of arrays. The default value is false to keep backward
> > > >> compatibility in the next Ignite release(2.11).
> > > >>>
> > > >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> Ignite
> > > >> releases (2.12).
> > > >>>
> > > >>> WDYT?
> > > >>>
> > > >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> > > >>
> > > >>
> > >
> > >
>
>
>
> --
> Sincerely yours, Ivan Daschinskiy
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ivan Daschinsky <iv...@gmail.com>.
Hi!
First of all, when array is serialized, marshaller actually DO
PRESERVE type of element (seel
org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray and
org.apache.ignite.internal.binary.BinaryWriterExImpl#doWriteObjectArray).
AFAIK, the motivation of Nickolay proposal, is the fact, that during
call of PlatformService we do additional marshal/unmarshall step and
during this step we loose array type info
See org.apache.ignite.internal.binary.BinaryReaderExImpl#readObjectDetached
and org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray)
In order to handle this problem, we can just add some wrapper that
contains element type info and use it in our INTERNAL API.
I just don't understand why we should change IgniteBinary API.
>> It was designed as a data storage format, and the fact
that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
something we're trying to fix in 3.0.
Please, do not. There are many cases when this can really improve
performance. Reflection is always slower than low level api and many
users are happy with low level API.
сб, 1 мая 2021 г. в 00:51, Valentin Kulichenko <va...@gmail.com>:
>
> Hi Nikolay,
>
> Is there a specific motivation behind your proposal? I do acknowledge that
> the semantics of the toBinary method is a little weird, but my concern is
> that the way it works with arrays is just an example. Are you suggesting
> changing collections, primitives, and other "first citizen" data types as
> well? To me, that looks like a huge risky change without a clear value.
>
> I actually believe that binary format *should not* be used as a universal
> serde mechanism. It was designed as a data storage format, and the fact
> that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
> something we're trying to fix in 3.0.
>
> That said, I'm not necessarily against the change (especially if we do not
> change the default behavior). But I would like to better understand its
> scope (e.g., arrays only or beyond?), as well as get some examples of use
> cases that would benefit from the change.
>
> -Val
>
>
> On Fri, Apr 30, 2021 at 7:35 AM Nikolay Izhikov <ni...@apache.org> wrote:
>
> > Hello, Ilya.
> >
> > Thanks for the feedback!
> >
> > > For me it sounds like something we would like to do in 3.0
> >
> > Ignite 3 is a very long way to go, so I prefer to target this fix in
> > Ignite 2.x.
> >
> > > I think making it default "true" is a breaking change and is not
> > possible in a minor release
> >
> > Yes, you are correct it is a breaking change.
> > It seems for me, we all agreed that breaking changes are possible in minor
> > releases.
> >
> > Anyway, if we will decide do not to enable this feature by default it’s OK
> > for me.
> > We still can implement it and improve the binary SerDe mechanism.
> >
> >
> > > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
> > написал(а):
> > >
> > > Hello!
> > >
> > > For me it sounds like something we would like to do in 3.0 (if indeed it
> > > will have arrays as possible value (or key) type), but doing it in 2.x
> > > raises concerns whether it has enough time left to stabilize.
> > >
> > > Also, I think making it default "true" is a breaking change and is not
> > > possible in a minor release, case in point,
> > > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
> > > destructive.
> > >
> > > Of course I would also like to hear what other community members think.
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> > >
> > >
> > > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> > >
> > >> Igniters,
> > >>
> > >> Want to clarify my proposal about new array store format.
> > >> I think we should store array in special binary wrapper that will keep
> > >> original component type
> > >>
> > >> ```
> > >> public class BinaryArrayWrapper implements BinaryObjectEx,
> > Externalizable {
> > >> /** Type ID. */
> > >> private int compTypeId;
> > >>
> > >> /** Raw data. */
> > >> private String compClsName;
> > >>
> > >> /** Value. */
> > >> private Object[] arr;
> > >>
> > >> // Further implementation.
> > >> }
> > >> ```
> > >>
> > >>
> > >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> > >> написал(а):
> > >>>
> > >>> Hello, Igniters.
> > >>>
> > >>> Currently, binary marshaller works as follows(Say, we have a class
> > >> `User` then):
> > >>>
> > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>> IgniteBinary#toBinary(User[])` -> Object[]
> > >>> IgniteBinary#toBinary(Object[])` -> Object[]
> > >>>
> > >>> This means, that we lose array component type information during binary
> > >> serialization.
> > >>> AFAIK, it’s a design choice made during binary infrastructure
> > >> development.
> > >>>
> > >>> This lead to the following disadvantages:
> > >>>
> > >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> > >>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
> > >> hacks to deal with custom user array and still has many issues [1]
> > >>>
> > >>> I propose to make breaking changes and fix the custom user array SeDe
> > as
> > >> follows:
> > >>>
> > >>> 1. Implement binary serialization that correctly Ser and Deser
> > >> array using some kind of the wrapper (BinaryArrayWrapper).
> > >>>
> > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> > >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> > >>>
> > >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> > >> correct SerDe of arrays. The default value is false to keep backward
> > >> compatibility in the next Ignite release(2.11).
> > >>>
> > >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> > >> releases (2.12).
> > >>>
> > >>> WDYT?
> > >>>
> > >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> > >>
> > >>
> >
> >
--
Sincerely yours, Ivan Daschinskiy
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Valentin Kulichenko <va...@gmail.com>.
Hi Nikolay,
Is there a specific motivation behind your proposal? I do acknowledge that
the semantics of the toBinary method is a little weird, but my concern is
that the way it works with arrays is just an example. Are you suggesting
changing collections, primitives, and other "first citizen" data types as
well? To me, that looks like a huge risky change without a clear value.
I actually believe that binary format *should not* be used as a universal
serde mechanism. It was designed as a data storage format, and the fact
that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
something we're trying to fix in 3.0.
That said, I'm not necessarily against the change (especially if we do not
change the default behavior). But I would like to better understand its
scope (e.g., arrays only or beyond?), as well as get some examples of use
cases that would benefit from the change.
-Val
On Fri, Apr 30, 2021 at 7:35 AM Nikolay Izhikov <ni...@apache.org> wrote:
> Hello, Ilya.
>
> Thanks for the feedback!
>
> > For me it sounds like something we would like to do in 3.0
>
> Ignite 3 is a very long way to go, so I prefer to target this fix in
> Ignite 2.x.
>
> > I think making it default "true" is a breaking change and is not
> possible in a minor release
>
> Yes, you are correct it is a breaking change.
> It seems for me, we all agreed that breaking changes are possible in minor
> releases.
>
> Anyway, if we will decide do not to enable this feature by default it’s OK
> for me.
> We still can implement it and improve the binary SerDe mechanism.
>
>
> > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > For me it sounds like something we would like to do in 3.0 (if indeed it
> > will have arrays as possible value (or key) type), but doing it in 2.x
> > raises concerns whether it has enough time left to stabilize.
> >
> > Also, I think making it default "true" is a breaking change and is not
> > possible in a minor release, case in point,
> > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
> > destructive.
> >
> > Of course I would also like to hear what other community members think.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> >
> >> Igniters,
> >>
> >> Want to clarify my proposal about new array store format.
> >> I think we should store array in special binary wrapper that will keep
> >> original component type
> >>
> >> ```
> >> public class BinaryArrayWrapper implements BinaryObjectEx,
> Externalizable {
> >> /** Type ID. */
> >> private int compTypeId;
> >>
> >> /** Raw data. */
> >> private String compClsName;
> >>
> >> /** Value. */
> >> private Object[] arr;
> >>
> >> // Further implementation.
> >> }
> >> ```
> >>
> >>
> >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> >> написал(а):
> >>>
> >>> Hello, Igniters.
> >>>
> >>> Currently, binary marshaller works as follows(Say, we have a class
> >> `User` then):
> >>>
> >>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>> IgniteBinary#toBinary(User[])` -> Object[]
> >>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>
> >>> This means, that we lose array component type information during binary
> >> serialization.
> >>> AFAIK, it’s a design choice made during binary infrastructure
> >> development.
> >>>
> >>> This lead to the following disadvantages:
> >>>
> >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
> >> hacks to deal with custom user array and still has many issues [1]
> >>>
> >>> I propose to make breaking changes and fix the custom user array SeDe
> as
> >> follows:
> >>>
> >>> 1. Implement binary serialization that correctly Ser and Deser
> >> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>
> >>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>
> >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >> correct SerDe of arrays. The default value is false to keep backward
> >> compatibility in the next Ignite release(2.11).
> >>>
> >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> >> releases (2.12).
> >>>
> >>> WDYT?
> >>>
> >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>
> >>
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Hello, Ilya.
Thanks for the feedback!
> For me it sounds like something we would like to do in 3.0
Ignite 3 is a very long way to go, so I prefer to target this fix in Ignite 2.x.
> I think making it default "true" is a breaking change and is not possible in a minor release
Yes, you are correct it is a breaking change.
It seems for me, we all agreed that breaking changes are possible in minor releases.
Anyway, if we will decide do not to enable this feature by default it’s OK for me.
We still can implement it and improve the binary SerDe mechanism.
> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <il...@gmail.com> написал(а):
>
> Hello!
>
> For me it sounds like something we would like to do in 3.0 (if indeed it
> will have arrays as possible value (or key) type), but doing it in 2.x
> raises concerns whether it has enough time left to stabilize.
>
> Also, I think making it default "true" is a breaking change and is not
> possible in a minor release, case in point,
> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
> destructive.
>
> Of course I would also like to hear what other community members think.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
>
>> Igniters,
>>
>> Want to clarify my proposal about new array store format.
>> I think we should store array in special binary wrapper that will keep
>> original component type
>>
>> ```
>> public class BinaryArrayWrapper implements BinaryObjectEx, Externalizable {
>> /** Type ID. */
>> private int compTypeId;
>>
>> /** Raw data. */
>> private String compClsName;
>>
>> /** Value. */
>> private Object[] arr;
>>
>> // Further implementation.
>> }
>> ```
>>
>>
>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
>> написал(а):
>>>
>>> Hello, Igniters.
>>>
>>> Currently, binary marshaller works as follows(Say, we have a class
>> `User` then):
>>>
>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>> IgniteBinary#toBinary(User[])` -> Object[]
>>> IgniteBinary#toBinary(Object[])` -> Object[]
>>>
>>> This means, that we lose array component type information during binary
>> serialization.
>>> AFAIK, it’s a design choice made during binary infrastructure
>> development.
>>>
>>> This lead to the following disadvantages:
>>>
>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
>> hacks to deal with custom user array and still has many issues [1]
>>>
>>> I propose to make breaking changes and fix the custom user array SeDe as
>> follows:
>>>
>>> 1. Implement binary serialization that correctly Ser and Deser
>> array using some kind of the wrapper (BinaryArrayWrapper).
>>>
>>> IgniteBinary#toBinary(User)` -> BinaryObject
>>> IgniteBinary#toBinary(User[])` -> BinaryObject
>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
>>>
>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
>> correct SerDe of arrays. The default value is false to keep backward
>> compatibility in the next Ignite release(2.11).
>>>
>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
>> releases (2.12).
>>>
>>> WDYT?
>>>
>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
>>
>>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
For me it sounds like something we would like to do in 3.0 (if indeed it
will have arrays as possible value (or key) type), but doing it in 2.x
raises concerns whether it has enough time left to stabilize.
Also, I think making it default "true" is a breaking change and is not
possible in a minor release, case in point,
IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
destructive.
Of course I would also like to hear what other community members think.
Regards,
--
Ilya Kasnacheev
пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <ni...@apache.org>:
> Igniters,
>
> Want to clarify my proposal about new array store format.
> I think we should store array in special binary wrapper that will keep
> original component type
>
> ```
> public class BinaryArrayWrapper implements BinaryObjectEx, Externalizable {
> /** Type ID. */
> private int compTypeId;
>
> /** Raw data. */
> private String compClsName;
>
> /** Value. */
> private Object[] arr;
>
> // Further implementation.
> }
> ```
>
>
> > 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com>
> написал(а):
> >
> > Hello, Igniters.
> >
> > Currently, binary marshaller works as follows(Say, we have a class
> `User` then):
> >
> > IgniteBinary#toBinary(User)` -> BinaryObject
> > IgniteBinary#toBinary(User[])` -> Object[]
> > IgniteBinary#toBinary(Object[])` -> Object[]
> >
> > This means, that we lose array component type information during binary
> serialization.
> > AFAIK, it’s a design choice made during binary infrastructure
> development.
> >
> > This lead to the following disadvantages:
> >
> > 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> > 2. Ignite internals(service grid, .Net calls) contains many tweaks and
> hacks to deal with custom user array and still has many issues [1]
> >
> > I propose to make breaking changes and fix the custom user array SeDe as
> follows:
> >
> > 1. Implement binary serialization that correctly Ser and Deser
> array using some kind of the wrapper (BinaryArrayWrapper).
> >
> > IgniteBinary#toBinary(User)` -> BinaryObject
> > IgniteBinary#toBinary(User[])` -> BinaryObject
> > IgniteBinary#toBinary(Object[])` -> BinaryObject
> >
> > 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> correct SerDe of arrays. The default value is false to keep backward
> compatibility in the next Ignite release(2.11).
> >
> > 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> releases (2.12).
> >
> > WDYT?
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-14299
>
>
Re: [DISCUSSION] Array to BinaryObject serialization
Posted by Nikolay Izhikov <ni...@apache.org>.
Igniters,
Want to clarify my proposal about new array store format.
I think we should store array in special binary wrapper that will keep original component type
```
public class BinaryArrayWrapper implements BinaryObjectEx, Externalizable {
/** Type ID. */
private int compTypeId;
/** Raw data. */
private String compClsName;
/** Value. */
private Object[] arr;
// Further implementation.
}
```
> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <ni...@gmail.com> написал(а):
>
> Hello, Igniters.
>
> Currently, binary marshaller works as follows(Say, we have a class `User` then):
>
> IgniteBinary#toBinary(User)` -> BinaryObject
> IgniteBinary#toBinary(User[])` -> Object[]
> IgniteBinary#toBinary(Object[])` -> Object[]
>
> This means, that we lose array component type information during binary serialization.
> AFAIK, it’s a design choice made during binary infrastructure development.
>
> This lead to the following disadvantages:
>
> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> 2. Ignite internals(service grid, .Net calls) contains many tweaks and hacks to deal with custom user array and still has many issues [1]
>
> I propose to make breaking changes and fix the custom user array SeDe as follows:
>
> 1. Implement binary serialization that correctly Ser and Deser array using some kind of the wrapper (BinaryArrayWrapper).
>
> IgniteBinary#toBinary(User)` -> BinaryObject
> IgniteBinary#toBinary(User[])` -> BinaryObject
> IgniteBinary#toBinary(Object[])` -> BinaryObject
>
> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables correct SerDe of arrays. The default value is false to keep backward compatibility in the next Ignite release(2.11).
>
> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite releases (2.12).
>
> WDYT?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-14299