[DISCUSSION] Array to BinaryObject serialization

[Nikolay Izhikov-2]

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

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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

[Ilya Kasnacheev]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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
>
>

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> 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
>>
>>

[Valentin Kulichenko]

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 <[hidden email]> 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 <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> >> написал(а):
> >>>
> >>> 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
> >>
> >>
>
>

[Ivan Daschinsky]

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 <[hidden email]>:

>
> 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 <[hidden email]> 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 <[hidden email]>
> > написал(а):
> > >
> > > 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 <[hidden email]>:
> > >
> > >> 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 <[hidden email]>
> > >> написал(а):
> > >>>
> > >>> 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

[Valentin Kulichenko]

Hi Ivan,

Thanks for chiming in. My comments are below.

-Val

On Sat, May 1, 2021 at 12:20 AM Ivan Daschinsky <[hidden email]> 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 <
> [hidden email]>:
> >
> > 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 <[hidden email]>
> 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 <[hidden email]
> >
> > > написал(а):
> > > >
> > > > 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 <[hidden email]>:
> > > >
> > > >> 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 <[hidden email]>
> > > >> написал(а):
> > > >>>
> > > >>> 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
>

[Pavel Tupitsyn]

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 <
[hidden email]> wrote:

> Hi Ivan,
>
> Thanks for chiming in. My comments are below.
>
> -Val
>
> On Sat, May 1, 2021 at 12:20 AM Ivan Daschinsky <[hidden email]>
> 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 <
> > [hidden email]>:
> > >
> > > 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 <[hidden email]>
> > 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 <
> [hidden email]
> > >
> > > > написал(а):
> > > > >
> > > > > 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 <[hidden email]
> >:
> > > > >
> > > > >> 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 <
> [hidden email]>
> > > > >> написал(а):
> > > > >>>
> > > > >>> 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
> >
>

[Ilya Kasnacheev]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> >> написал(а):
> >>>
> >>> 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
> >>
> >>
>
>

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> 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 <[hidden email]>:
>>>
>>>> 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 <[hidden email]>
>>>> написал(а):
>>>>>
>>>>> 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
>>>>
>>>>
>>
>>

[Ilya Kasnacheev]

Hello!

Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
Fields.

Regards,
--
Ilya Kasnacheev


ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> >> написал(а):
> >>>
> >>> 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 <[hidden email]>:
> >>>
> >>>> 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 <[hidden email]>
> >>>> написал(а):
> >>>>>
> >>>>> 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
> >>>>
> >>>>
> >>
> >>
>
>

[Nikolay Izhikov-2]

Thanks, Ilya.

Can you put more context on this?
I don’t familiar with these issues.

> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]> написал(а):
>
> Hello!
>
> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> Fields.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> 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 <[hidden email]>:
>>>
>>>> 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 <[hidden email]>
>>>> написал(а):
>>>>>
>>>>> 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 <[hidden email]>:
>>>>>
>>>>>> 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 <[hidden email]>
>>>>>> написал(а):
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> Thanks, Ilya.
>
> Can you put more context on this?
> I don’t familiar with these issues.
>
>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]> написал(а):
>>
>> Hello!
>>
>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>> Fields.
>>
>> Regards,
>> --
>> Ilya Kasnacheev
>>
>>
>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
>>
>>> 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 <[hidden email]>
>>> написал(а):
>>>>
>>>> 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 <[hidden email]>:
>>>>
>>>>> 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 <[hidden email]>
>>>>> написал(а):
>>>>>>
>>>>>> 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 <[hidden email]>:
>>>>>>
>>>>>>> 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 <[hidden email]>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>

[Ilya Kasnacheev]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > Thanks, Ilya.
> >
> > Can you put more context on this?
> > I don’t familiar with these issues.
> >
> >> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]>
> написал(а):
> >>
> >> Hello!
> >>
> >> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> >> Fields.
> >>
> >> Regards,
> >> --
> >> Ilya Kasnacheev
> >>
> >>
> >> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
> >>
> >>> 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 <[hidden email]>
> >>> написал(а):
> >>>>
> >>>> 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 <[hidden email]>:
> >>>>
> >>>>> 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 <
> [hidden email]>
> >>>>> написал(а):
> >>>>>>
> >>>>>> 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 <[hidden email]>:
> >>>>>>
> >>>>>>> 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 <[hidden email]
> >
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> 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
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> Thanks, Ilya.
>>>
>>> Can you put more context on this?
>>> I don’t familiar with these issues.
>>>
>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]>
>> написал(а):
>>>>
>>>> Hello!
>>>>
>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>>>> Fields.
>>>>
>>>> Regards,
>>>> --
>>>> Ilya Kasnacheev
>>>>
>>>>
>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
>>>>
>>>>> 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 <[hidden email]>
>>>>> написал(а):
>>>>>>
>>>>>> 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 <[hidden email]>:
>>>>>>
>>>>>>> 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 <
>> [hidden email]>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> 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 <[hidden email]>:
>>>>>>>>
>>>>>>>>> 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 <[hidden email]
>>>
>>>>>>>>> написал(а):
>>>>>>>>>>
>>>>>>>>>> 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
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>
>>

[Ilya Kasnacheev]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> >> написал(а):
> >>>
> >>> Thanks, Ilya.
> >>>
> >>> Can you put more context on this?
> >>> I don’t familiar with these issues.
> >>>
> >>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]>
> >> написал(а):
> >>>>
> >>>> Hello!
> >>>>
> >>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> >>>> Fields.
> >>>>
> >>>> Regards,
> >>>> --
> >>>> Ilya Kasnacheev
> >>>>
> >>>>
> >>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
> >>>>
> >>>>> 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 <[hidden email]>
> >>>>> написал(а):
> >>>>>>
> >>>>>> 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 <[hidden email]>:
> >>>>>>
> >>>>>>> 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 <
> >> [hidden email]>
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> 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 <[hidden email]
> >:
> >>>>>>>>
> >>>>>>>>> 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 <
> [hidden email]
> >>>
> >>>>>>>>> написал(а):
> >>>>>>>>>>
> >>>>>>>>>> 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
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> 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 <[hidden email]>:
>>>
>>>> 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 <[hidden email]>
>>>> написал(а):
>>>>>
>>>>> Thanks, Ilya.
>>>>>
>>>>> Can you put more context on this?
>>>>> I don’t familiar with these issues.
>>>>>
>>>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]>
>>>> написал(а):
>>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
>>>>>> Fields.
>>>>>>
>>>>>> Regards,
>>>>>> --
>>>>>> Ilya Kasnacheev
>>>>>>
>>>>>>
>>>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
>>>>>>
>>>>>>> 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 <[hidden email]>
>>>>>>> написал(а):
>>>>>>>>
>>>>>>>> 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 <[hidden email]>:
>>>>>>>>
>>>>>>>>> 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 <
>>>> [hidden email]>
>>>>>>>>> написал(а):
>>>>>>>>>>
>>>>>>>>>> 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 <[hidden email]
>>> :
>>>>>>>>>>
>>>>>>>>>>> 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 <
>> [hidden email]
>>>>>
>>>>>>>>>>> написал(а):
>>>>>>>>>>>>
>>>>>>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>>
>>
>>

[Ilya Kasnacheev]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> >> написал(а):
> >>>
> >>> 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 <[hidden email]>:
> >>>
> >>>> 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 <[hidden email]>
> >>>> написал(а):
> >>>>>
> >>>>> Thanks, Ilya.
> >>>>>
> >>>>> Can you put more context on this?
> >>>>> I don’t familiar with these issues.
> >>>>>
> >>>>>> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <[hidden email]
> >
> >>>> написал(а):
> >>>>>>
> >>>>>> Hello!
> >>>>>>
> >>>>>> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary
> Object
> >>>>>> Fields.
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Ilya Kasnacheev
> >>>>>>
> >>>>>>
> >>>>>> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <[hidden email]>:
> >>>>>>
> >>>>>>> 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 <
> [hidden email]>
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> 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 <[hidden email]
> >:
> >>>>>>>>
> >>>>>>>>> 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 <
> >>>> [hidden email]>
> >>>>>>>>> написал(а):
> >>>>>>>>>>
> >>>>>>>>>> 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 <
> [hidden email]
> >>> :
> >>>>>>>>>>
> >>>>>>>>>>> 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 <
> >> [hidden email]
> >>>>>
> >>>>>>>>>>> написал(а):
> >>>>>>>>>>>>
> >>>>>>>>>>>> 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
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>