Hello, Pavel. Agree. Will implement it.
> 13 янв. 2021 г., в 20:28, Pavel Tupitsyn <ptupit...@apache.org> написал(а): > >> I propose to implicitly register classes only in the case they are sent > to the Java side. > > Ok, I guess we can do this when all of the following is true > - We are inside ExecuteJavaTask or Java service call > - FQN mapper is used (default or explicit) > - There is no BinaryIdMapper on .NET or Java side > > > On Wed, Jan 13, 2021 at 6:30 PM Nikolay Izhikov <nizhi...@apache.org> wrote: > >> Pavel. >> >>> Yes, but they are registered separately for .NET and Java, please see >> how MarshallerContextImpl stores typeId->typeName mappings. >> >> This will not change. We still will have separate type registration. >> >>> With the proposal we put all class names in the same pile. >> >> Implementation of type registration will not be changed. >> What I propose is to simplify user life in the exact places we can do it >> for free. >> >>> Now imagine that the user has a cluster with C# and Java clients. >>> C# client uses class "foo.bar.Aa" for Compute, >>> Java client uses class "foo.bar.BB" for Services - independent use >> cases, >>> all is well. >> >> I propose to implicitly register classes only in the case they are sent to >> the Java side. >> So, for all classes that is used only in .Net nothing will change. >> >> >>> 13 янв. 2021 г., в 15:14, Pavel Tupitsyn <ptupit...@apache.org> >> написал(а): >>> >>>> Classes MUST be registered to be used in compute or cache. >>> >>> Yes, but they are registered separately for .NET and Java, >>> please see how MarshallerContextImpl stores typeId->typeName mappings. >>> >>> With the proposal we put all class names in the same pile. >>> >>> Now imagine that the user has a cluster with C# and Java clients. >>> C# client uses class "foo.bar.Aa" for Compute, >>> Java client uses class "foo.bar.BB" for Services - independent use >> cases, >>> all is well. >>> >>> As soon as we put all class names in the same map, the app breaks, >>> because "foo.bar.Aa" and "foo.bar.BB" have the same hash code, >>> and DuplicateTypeIdException is thrown by Ignite. >>> >>> On Wed, Jan 13, 2021 at 1:54 PM Nikolay Izhikov <nizhi...@apache.org> >> wrote: >>> >>>> Hello, Pavel. >>>> >>>>> Imagine that some Ignite user has lots of .NET and Java classes being >>>> stored in cache, used in compute, >>>> >>>> Classes MUST be registered to be used in compute or cache. >>>> So, in your example, user registered classes by hand, already. >>>> >>>>> Let's add a flag like BinaryConfiguration.AssumeSameJavaTypeNames, >>>> >>>> We already have this «flag». >>>> It a default INameMapper implementation that assumes we use the same >> class >>>> names both in Java and .Net. >>>> >>>>> This is not up for discussion, we do not break compatibility like this. >>>> >>>> Sorry, I don’t see compatibility issues in the proposal. >>>> Can you, please, provide an example? >>>> >>>>> 13 янв. 2021 г., в 13:46, Pavel Tupitsyn <ptupit...@apache.org> >>>> написал(а): >>>>> >>>>> Nikolay, >>>>> >>>>> I agree with your proposal, with one addition: >>>>> this behavior must be disabled by default for compatibility reasons. >>>>> >>>>> Imagine that some Ignite user has lots of .NET and Java classes being >>>>> stored in cache, >>>>> used in compute, etc. Now with the proposal implemented, all .NET >> classes >>>>> are registered >>>>> as Java classes too, which has a potential for hash code collisions, >>>>> resulting in DuplicateTypeIdException. >>>>> >>>>> So the user can't upgrade to 2.11, their app does not work anymore, >> which >>>>> is not acceptable. >>>>> This is not up for discussion, we do not break compatibility like this. >>>>> >>>>> Let's add a flag like BinaryConfiguration.AssumeSameJavaTypeNames, >>>>> which enables the behavior globally and both ways: >>>>> all type names become shared across all platforms in >>>> MarshallerContextImpl. >>>>> >>>>> >>>>> On Wed, Jan 13, 2021 at 11:21 AM Nikolay Izhikov <nizhi...@apache.org> >>>>> wrote: >>>>> >>>>>> Hello, Pavel >>>>>> >>>>>> My proposal is the following: >>>>>> >>>>>> *When Ignite configured with FQN NameMapper.* >>>>>> And user invokes >>>>>> >>>>>> 1. Service method from .Net to Java. >>>>>> 2. Compute methods `ExecuteJavaTask`, `ExecuteJavaTaskAsync` >>>>>> 3. Cache operations. >>>>>> >>>>>> Ignite should implicitly register types both for .Net and Java >> platform. >>>>>> >>>>>> ===== >>>>>> My intentions is to simplify usage of the Ignite .Net platform by >>>>>> eliminating necessity of BinaryConfiguration in .Net. >>>>>> >>>>>> Approach when user should modify Ignite configuration on both sides >> Java >>>>>> and .Net every time he use new class as a service parameter looks >> ugly, >>>> for >>>>>> me. >>>>>> >>>>>> You can see an example of my idea in services test [1] >>>>>> In current release, user forced to enlist each type in configuration. >>>>>> Now, you just deploy Java service and use it. >>>>>> >>>>>> Please, Note: >>>>>> >>>>>> 1. FQN NameMapper is a default value. >>>>>> 2. FQN NameMapper requires from the user to have exactly same names >> for >>>>>> custom binary types in Java and .Net. >>>>>> 3. Improvals for Services in master, already. >>>>>> >>>>>> [1] >>>>>> >>>> >> https://github.com/apache/ignite/blob/master/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Services/ServiceTypeAutoResolveTest.cs#L146 >>>>>> >>>>>>> 13 янв. 2021 г., в 10:13, Pavel Tupitsyn <ptupit...@apache.org> >>>>>> написал(а): >>>>>>> >>>>>>> Nikolay, >>>>>>> >>>>>>> As I understand, you insist on changing the default behavior, is this >>>>>>> correct? >>>>>>> >>>>>>> >>>>>>> On Tue, Jan 12, 2021 at 9:51 PM Nikolay Izhikov <nizhi...@apache.org >>> >>>>>> wrote: >>>>>>> >>>>>>>> Hello, Igor, Pavel. >>>>>>>> >>>>>>>> Thanks for the feedback >>>>>>>> >>>>>>>>> Agree with Pavel, this should be disabled by default. >>>>>>>> >>>>>>>> Right now, Ignite, by default, forcing users to enlist every single >>>>>> value >>>>>>>> object they have in project in the config. >>>>>>>> Do you think it’s a right way to go? >>>>>>>> >>>>>>>>> To me it looks pretty dangerous as users do not explicitly control >>>>>>>> what’s going to be registered and it could lead to hard-to-debug >>>>>> mistakes >>>>>>>> when wrong classes get registered or with wrong names. >>>>>>>> >>>>>>>> Approach with transparent type registration implemented in every RPC >>>>>>>> framework I know. >>>>>>>> I think user expect it from the modern software. >>>>>>>> Can you, please, highlight dangerous part of the approach? >>>>>>>> >>>>>>>>> Wouldn't >>>>>>>>> it be more reasonable to only register those which are used with >> Java >>>>>>>>> services? >>>>>>>> >>>>>>>> Correct. Will do it. >>>>>>>> >>>>>>>>> Registering every .NET type as a Java type can lead to typeId >>>>>> collisions >>>>>>>> and break existing user code, >>>>>>>> >>>>>>>> For Service and Compute API binary type registration both in Java >> and >>>>>> .Net >>>>>>>> will happen anyway. >>>>>>>> It’s required just to get things work. >>>>>>>> >>>>>>>>> 12 янв. 2021 г., в 20:31, Igor Sapego <isap...@apache.org> >>>> написал(а): >>>>>>>>> >>>>>>>>> Agree with Pavel, this should be disabled by default. >>>>>>>>> >>>>>>>>> To me it looks pretty dangerous as users do not explicitly control >>>>>> what's >>>>>>>>> going to be >>>>>>>>> registered and it could lead to hard-to-debug mistakes when wrong >>>>>> classes >>>>>>>>> get >>>>>>>>> registered or with wrong names. Also it can be hard to use with >>>> classes >>>>>>>>> that should >>>>>>>>> not be registered at all, which is often the case because not >>>> everyone >>>>>>>> use >>>>>>>>> .NET client >>>>>>>>> with Java services. >>>>>>>>> >>>>>>>>> To me it looks familiar to another of our features - peer class >>>>>> loading, >>>>>>>>> which is disabled >>>>>>>>> by default for similar reasons. >>>>>>>>> >>>>>>>>> Also, why register types which are used as method arguments for any >>>>>>>>> service? Wouldn't >>>>>>>>> it be more reasonable to only register those which are used with >> Java >>>>>>>>> services? >>>>>>>>> >>>>>>>>> Best Regards, >>>>>>>>> Igor >>>>>>>>> >>>>>>>>> >>>>>>>>> On Tue, Jan 12, 2021 at 7:35 PM Pavel Tupitsyn < >> ptupit...@apache.org >>>>> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Nikolay, >>>>>>>>>> >>>>>>>>>> I think this behavior should be opt-in - let's add a flag to >>>>>>>>>> BinaryConfiguration. >>>>>>>>>> Registering every .NET type as a Java type can lead to typeId >>>>>> collisions >>>>>>>>>> and break existing user code, >>>>>>>>>> so we can't enable this by default. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ignite stores typeId -> className mappings separately for Java and >>>>>> .NET >>>>>>>>>> [1]. >>>>>>>>>> Separate maps were introduced because C# and Java have different >>>>>> naming >>>>>>>>>> conventions, >>>>>>>>>> typically you have org.foo.bar.Person in Java and Foo.Bar.Person >> in >>>>>>>> .NET, >>>>>>>>>> so we allow the users to map two different type names to the same >>>>>> typeId >>>>>>>>>> with a custom name or id mapper. >>>>>>>>>> >>>>>>>>>> This proposal says we should always put Foo.Bar.Person from .NET >> to >>>>>> the >>>>>>>>>> Java mapping, >>>>>>>>>> in the hope that there is a class with that name in Java, which >> is a >>>>>>>> very >>>>>>>>>> rare use case. >>>>>>>>>> >>>>>>>>>> [1] >>>>>>>>>> >>>>>> >> org.apache.ignite.internal.MarshallerContextImpl#registerClassName(byte, >>>>>>>>>> int, java.lang.String, boolean) >>>>>>>>>> >>>>>>>>>> On Tue, Jan 12, 2021 at 4:56 PM Nikolay Izhikov < >>>> nizhi...@apache.org> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hello, Igniters. >>>>>>>>>>> >>>>>>>>>>> Currently, in case of usage .Net platform client it’s required >> for >>>>>> the >>>>>>>>>>> user to explicitly register each custom type. >>>>>>>>>>> >>>>>>>>>>> ``` >>>>>>>>>>> _marsh = new Marshaller(new BinaryConfiguration >>>>>>>>>>> { >>>>>>>>>>> TypeConfigurations = new List<BinaryTypeConfiguration> >>>>>>>>>>> { >>>>>>>>>>> new BinaryTypeConfiguration(typeof (Address)), >>>>>>>>>>> new BinaryTypeConfiguration(typeof (TestModel)) >>>>>>>>>>> } >>>>>>>>>>> }); >>>>>>>>>>> ``` >>>>>>>>>>> >>>>>>>>>>> Note, we have a special public interface to map java type to .net >>>>>>>> types - >>>>>>>>>>> `IBinaryNameMapper` >>>>>>>>>>> >>>>>>>>>>> ``` >>>>>>>>>>> public interface IBinaryNameMapper >>>>>>>>>>> { >>>>>>>>>>> string GetTypeName(string name); >>>>>>>>>>> string GetFieldName(string name); >>>>>>>>>>> } >>>>>>>>>>> } >>>>>>>>>>> ``` >>>>>>>>>>> >>>>>>>>>>> Users found this approach annoying and not convenient. >>>>>>>>>>> So, recently we made a several improvements for the Service API >> to >>>>>>>>>>> implicitly register each custom type from service method >> arguments. >>>>>>>> [1], >>>>>>>>>>> [2], [3] >>>>>>>>>>> For now, user can just call Java service from .Net without any >>>>>>>> additional >>>>>>>>>>> configuration in case FQN NameMapper used. >>>>>>>>>>> >>>>>>>>>>> I propose to extends approach with the implicit binary type >>>>>>>> registration >>>>>>>>>>> to all APIs and prepare PR for it [4]. >>>>>>>>>>> >>>>>>>>>>> What do you think? >>>>>>>>>>> >>>>>>>>>>> [1] >>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >> https://github.com/apache/ignite/commit/35f551c023a12b3570d65c803d10c89480f7d5e4 >>>>>>>>>>> [2] >>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >> https://github.com/apache/ignite/commit/6d02e32e7f049e4f78f7abd37f4ff91d77f738c2 >>>>>>>>>>> [3] >>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >> https://github.com/apache/ignite/commit/c2204cda29e70294cc93756eabc844b64e07a42e >>>>>>>>>>> [4] https://github.com/apache/ignite/pull/8635 >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>> >>>> >> >>
