[DISCUSSION] .Net BinaryTypes transparency
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
{
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
Re: [DISCUSSION] .Net BinaryTypes transparency
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 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
> {
> 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
Re: [DISCUSSION] .Net BinaryTypes transparency
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 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
> 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
> > {
> > 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
>
Re: [DISCUSSION] .Net BinaryTypes transparency
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 написал(а):
>
> 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 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
>> 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
>>>{
>>>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
>>
Re: [DISCUSSION] .Net BinaryTypes transparency
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 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 написал(а):
> >
> > 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
> 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
> >> 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
> >>>{
> >>>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
> >>
>
>
Re: [DISCUSSION] .Net BinaryTypes transparency
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 написал(а):
>
> 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 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 написал(а):
>>>
>>> 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
>> 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
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
> {
> new BinaryTypeConfiguration(typeof (Address)),
> new BinaryTypeConfiguration(typeof (TestModel))
> }
> });
> ```
>
> Note, we have a special public interface to map java type to .net
>> types -
> `IBi
Re: [DISCUSSION] .Net BinaryTypes transparency
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 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 > написал(а): > > > > 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 > 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 написал(а): > >>> > >>> 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 > >> 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, > >
Re: [DISCUSSION] .Net BinaryTypes transparency
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 написал(а): > > 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 > 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 >> написал(а): >>> >>> 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 >> 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 написал(а): > > 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 > servi
Re: [DISCUSSION] .Net BinaryTypes transparency
> 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 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 > написал(а): > > > > 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 > > 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 > >> написал(а): > >>> > >>> 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 > >> 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 reg
Re: [DISCUSSION] .Net BinaryTypes transparency
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 написал(а): > >> 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 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 >> написал(а): >>> >>> 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 >>> 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 написал(а): > > 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 wrote: > >> Hello, Igor, Pavel. >> >> Thanks for the feedback >> >>> Agree with Pavel, this should be disabled by default. >> >> Right now, Ignite, by default, forcing
Re: [DISCUSSION] .Net BinaryTypes transparency
> 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 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 > написал(а): > > > >> 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 > 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 > >> написал(а): > >>> > >>> 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 > >>> 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/
Re: [DISCUSSION] .Net BinaryTypes transparency
Hello, Pavel. Agree. Will implement it. > 13 янв. 2021 г., в 20:28, Pavel Tupitsyn написал(а): > >> 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 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 >> написал(а): >>> 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 >> 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 написал(а): > > 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 > 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 th
