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.kasnach...@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 <nizhi...@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.kasnach...@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 <nizhi...@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....@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
>>>> 
>>>> 
>> 
>> 

Reply via email to