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 <ilya.kasnach...@gmail.com> > написал(а): > > Hello! > > Why do you need to take compatibility into account here at all? > You can start writing entries to cache in new format while reading both old > and new format. > > I consider returning Object[] instead of ConcreteType[] a bug so it's > totally OK to start returning the "better" ConcreteType[] instead. > > I think you can just fix the bug while preserving accessibility of old > (pre-bugfix) data as it was pre-2.11. > > Regards, > -- > Ilya Kasnacheev > > > ср, 19 мая 2021 г. в 14:08, Nikolay Izhikov <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 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> >>>>> >>>> >>>> >> >>
