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 <nizhi...@apache.org>:
> Igniters.
>
> Just to clarify the issue:
>
> ```
> public class BinaryObjectTest extends GridCommonAbstractTest {
> /** */
> @Test
> public void testArray() throws Exception {
> Ignite ign = startGrid();
>
> IgniteCache<Integer, TestClass1[]> cache =
> ign.createCache("my-cache");
>
> cache.put(1, new TestClass1[] {new TestClass1(), new
> TestClass1()});
> TestClass1[] obj = cache.get(1); //This will fail with
> ClassCastException.
>
> assertEquals(TestClass1[].class, obj.getClass());
> }
> }
> ```
>
> > 19 мая 2021 г., в 13:04, Nikolay Izhikov <nizhikov....@gmail.com>
> написал(а):
> >
> > Thanks, Ilya.
> >
> > Can you put more context on this?
> > I don’t familiar with these issues.
> >
> >> 19 мая 2021 г., в 13:02, Ilya Kasnacheev <ilya.kasnach...@gmail.com>
> написал(а):
> >>
> >> Hello!
> >>
> >> Obvious issues are Lazy SQL, Event Driven Services, Sort Binary Object
> >> Fields.
> >>
> >> Regards,
> >> --
> >> Ilya Kasnacheev
> >>
> >>
> >> ср, 19 мая 2021 г. в 12:56, Nikolay Izhikov <nizhi...@apache.org>:
> >>
> >>> Hello,
> >>>
> >>>> However, for internal platform and services implementations we should
> >>> fix the root cause:
> >>>> avoid extra deserialization->serialization pass completely.
> >>>> This will also improve performance.
> >>>
> >>> Pavel, thanks for the feedback.
> >>> If I understand correctly, your suggestion is to know data size at the
> >>> start of reading service parameters.
> >>> Is it correct?
> >>>
> >>> Right now, when the service method invoked we pass an array of
> parameters
> >>> through platform reader/writer machinery.
> >>> On java side parameters read one by one and we don’t know the size of
> the
> >>> data on the read start.
> >>>
> >>> AFAICU, this will require x2 memory on the .Net or thin client-side.
> >>>
> >>>
> >>>
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java#L289
> >>>
> >>>
> >>>> if we are to break compatibility, I would like to see it done for some
> >>> really common pain point.
> >>>
> >>> Ilya, can you, please, provide a list of common issues with Ignite that
> >>> can be resolved
> >>> only with compatibility breakage?
> >>>
> >>>> 4 мая 2021 г., в 12:58, Ilya Kasnacheev <ilya.kasnach...@gmail.com>
> >>> написал(а):
> >>>>
> >>>> Hello!
> >>>>
> >>>> If we really decide to break some compatibility then Array to
> >>> BinaryObject
> >>>> serialization will be very, very low on my personal list.
> >>>>
> >>>> I just don't see how this issue is relevant. I have been reading and
> >>>> answering user list for a few years now, and I don't remember a single
> >>>> question about storing ConcreteType[] as value and complaining about
> type
> >>>> information loss.
> >>>>
> >>>> If you have a good scenario how do we keep persistent store binary
> >>>> compatibility here, without adding a lot of new code and still
> checking
> >>> for
> >>>> both old and new approaches - you can go forward for sure.
> >>>>
> >>>> However, it does seem questionable where we have a new wrapper class
> >>>> specifically for top level arrays. You can have this wrapper in your
> own
> >>>> client code and it should work OK.
> >>>>
> >>>> Bottom line, if we are to break compatibility, I would like to see it
> >>> done
> >>>> for some really common pain point.
> >>>>
> >>>> Regards,
> >>>> --
> >>>> Ilya Kasnacheev
> >>>>
> >>>>
> >>>> пт, 30 апр. 2021 г. в 17:34, Nikolay Izhikov <nizhi...@apache.org>:
> >>>>
> >>>>> Hello, Ilya.
> >>>>>
> >>>>> Thanks for the feedback!
> >>>>>
> >>>>>> For me it sounds like something we would like to do in 3.0
> >>>>>
> >>>>> Ignite 3 is a very long way to go, so I prefer to target this fix in
> >>>>> Ignite 2.x.
> >>>>>
> >>>>>> I think making it default "true" is a breaking change and is not
> >>>>> possible in a minor release
> >>>>>
> >>>>> Yes, you are correct it is a breaking change.
> >>>>> It seems for me, we all agreed that breaking changes are possible in
> >>> minor
> >>>>> releases.
> >>>>>
> >>>>> Anyway, if we will decide do not to enable this feature by default
> it’s
> >>> OK
> >>>>> for me.
> >>>>> We still can implement it and improve the binary SerDe mechanism.
> >>>>>
> >>>>>
> >>>>>> 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <
> ilya.kasnach...@gmail.com>
> >>>>> написал(а):
> >>>>>>
> >>>>>> Hello!
> >>>>>>
> >>>>>> For me it sounds like something we would like to do in 3.0 (if
> indeed
> >>> it
> >>>>>> will have arrays as possible value (or key) type), but doing it in
> 2.x
> >>>>>> raises concerns whether it has enough time left to stabilize.
> >>>>>>
> >>>>>> Also, I think making it default "true" is a breaking change and is
> not
> >>>>>> possible in a minor release, case in point,
> >>>>>> IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is
> >>> less
> >>>>>> destructive.
> >>>>>>
> >>>>>> Of course I would also like to hear what other community members
> think.
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Ilya Kasnacheev
> >>>>>>
> >>>>>>
> >>>>>> пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <nizhi...@apache.org>:
> >>>>>>
> >>>>>>> Igniters,
> >>>>>>>
> >>>>>>> Want to clarify my proposal about new array store format.
> >>>>>>> I think we should store array in special binary wrapper that will
> keep
> >>>>>>> original component type
> >>>>>>>
> >>>>>>> ```
> >>>>>>> public class BinaryArrayWrapper implements BinaryObjectEx,
> >>>>> Externalizable {
> >>>>>>> /** Type ID. */
> >>>>>>> private int compTypeId;
> >>>>>>>
> >>>>>>> /** Raw data. */
> >>>>>>> private String compClsName;
> >>>>>>>
> >>>>>>> /** Value. */
> >>>>>>> private Object[] arr;
> >>>>>>>
> >>>>>>> // Further implementation.
> >>>>>>> }
> >>>>>>> ```
> >>>>>>>
> >>>>>>>
> >>>>>>>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <nizhikov....@gmail.com
> >
> >>>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> Hello, Igniters.
> >>>>>>>>
> >>>>>>>> Currently, binary marshaller works as follows(Say, we have a class
> >>>>>>> `User` then):
> >>>>>>>>
> >>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(User[])` -> Object[]
> >>>>>>>> IgniteBinary#toBinary(Object[])` -> Object[]
> >>>>>>>>
> >>>>>>>> This means, that we lose array component type information during
> >>> binary
> >>>>>>> serialization.
> >>>>>>>> AFAIK, it’s a design choice made during binary infrastructure
> >>>>>>> development.
> >>>>>>>>
> >>>>>>>> This lead to the following disadvantages:
> >>>>>>>>
> >>>>>>>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> >>>>>>>> 2. Ignite internals(service grid, .Net calls) contains many tweaks
> >>> and
> >>>>>>> hacks to deal with custom user array and still has many issues [1]
> >>>>>>>>
> >>>>>>>> I propose to make breaking changes and fix the custom user array
> SeDe
> >>>>> as
> >>>>>>> follows:
> >>>>>>>>
> >>>>>>>> 1. Implement binary serialization that correctly Ser and Deser
> >>>>>>> array using some kind of the wrapper (BinaryArrayWrapper).
> >>>>>>>>
> >>>>>>>> IgniteBinary#toBinary(User)` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(User[])` -> BinaryObject
> >>>>>>>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> >>>>>>>>
> >>>>>>>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> >>>>>>> correct SerDe of arrays. The default value is false to keep
> backward
> >>>>>>> compatibility in the next Ignite release(2.11).
> >>>>>>>>
> >>>>>>>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing
> Ignite
> >>>>>>> releases (2.12).
> >>>>>>>>
> >>>>>>>> WDYT?
> >>>>>>>>
> >>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>
