> 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 > >>>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
