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