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 <nizhi...@apache.org>:
> Ilya,
>
> > Maybe we should just automate that, e.g., whenever user stores Type[],
> always store one-field ArrayWrapper object, and automatically unwrap it on
> get()
>
> Yes, I’m talking about this opportunity from the beginning of the thread.
>
> > 1. Implement binary serialization that correctly Ser and Deser array
> using some kind of the wrapper (BinaryArrayWrapper).
>
>
> > 19 мая 2021 г., в 14:05, Ilya Kasnacheev <ilya.kasnach...@gmail.com>
> написал(а):
> >
> > Hello!
> >
> > Yes, it does not look pretty, I agree. But I'm saying that I've not
> > encountered this issue on the user list before, and it can probably be
> > easily countered by storing some one-field ArrayWrapper object in the
> cache
> > as value.
> >
> > Maybe we should just automate that, e.g., whenever user stores Type[],
> > always store one-field ArrayWrapper object, and automatically unwrap it
> on
> > get()
> > This way we may not even need any changes to storage format, if binary
> > marshaller does not mind.
> >
> > Regards,
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 19 мая 2021 г. в 13:33, Nikolay Izhikov <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
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>
