Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Vladimir Ozerov]

The ticket is on the radar, but not in immediate plans. The problem might
sounds simple at first glance, but we already spent considerable time on
implementation and review, because we heavily rely on classes caching, and
a lot of internal BinaryMarshaller infrastructure should be reworked to
allow for this change. Hopefully, we will have it in Apache Ignite 2.5.

On Thu, Jan 4, 2018 at 2:28 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Ticket is still open. Vladimir, looks like it's assigned to you. Do you
> have any plans to work on it?
>
> https://issues.apache.org/jira/browse/IGNITE-5038
>
> -Val
>
> On Wed, Jan 3, 2018 at 1:26 PM, Abeneazer Chafamo <
> chaf...@nodalexchange.com> wrote:
>
>> Is there any update on the suggested functionality to resolve cache entry
>> classes based on the caller's context first instead of relying on Ignite's
>> classloader?
>>
>>
>>
>> --
>> Sent from: http://apache-ignite-users.70518.x6.nabble.com/
>>
>
>

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Valentin Kulichenko]

Ticket is still open. Vladimir, looks like it's assigned to you. Do you
have any plans to work on it?

https://issues.apache.org/jira/browse/IGNITE-5038

-Val

On Wed, Jan 3, 2018 at 1:26 PM, Abeneazer Chafamo  wrote:

> Is there any update on the suggested functionality to resolve cache entry
> classes based on the caller's context first instead of relying on Ignite's
> classloader?
>
>
>
> --
> Sent from: http://apache-ignite-users.70518.x6.nabble.com/
>

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Valentin Kulichenko]

Hi Nick,

How exactly do you replace the class loader and can you give a little bit
more detail about why you do this?

As for the issues, here are my comments:

   1. Can you clarify this? In which scenario it doesn't work and why?
   2. Why do you deploy domain classes in the first place? Ignite
   architecture assumes that there are no classes on server side at all, so I
   think it would be very hard to load them dynamically. Can you use binary
   objects instead?
   3. I can't recall any such places from the top of my head, but frankly I
   don't think you have such a guarantee. IgniteConfiguration object is
   assumed to be created prior to node startup. It should not be modified
   after startup.

-Val

On Thu, Jun 1, 2017 at 9:46 AM, npordash  wrote:

> I wanted to provide an update on where I am at with this as I'm still
> exploring different options.
>
> For the time being I've taken the path of using a delegating ClassLoader in
> IgniteConfiguration as was previously suggested; however, with the
> difference being that whenever a service is deployed/undeployed I end up
> replacing the ClassLoader with a new instance that has the service's
> ClassLoader added/removed. The replacement is necessary so that unused/old
> classes can be reclaimed by the garbage collector and that new versions can
> be deployed in the future.
>
> Overall I think this is a more comprehensive approach since it also allows
> execution constructs like CacheEntryProcessor implementations provided by
> the service to be used across the grid without enabling p2p classloading
> (assuming the service is deployed to every node).
>
> There are a few concerns/issues with this approach:
>
> 1) There are still isolation concerns with class versions across different
> services, but as long as the services don't have dependencies between each
> other or have overlapping class usage in a cache or execution context then
> it's a non-issue.
> 2) Using OnheapCacheEnabled in conjunction with service provided domain
> classes is impossible since once the service is reloaded and we get a new
> classloader all calls to deserialize will fail.
> 3) This approach only works if nothing caches the result of
> IgniteConfiguration.getClassLoader, which isn't obvious since the docs
> related to classloader behavior in Ignite is pretty sparse (or at least I
> could not find it). I don't think anything does when I check that method
> usage, but I'm not 100% sure about that.
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-
> deserializeValue-with-specific-ClassLoader-tp17126p18358.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[npordash]

I wanted to provide an update on where I am at with this as I'm still
exploring different options.

For the time being I've taken the path of using a delegating ClassLoader in
IgniteConfiguration as was previously suggested; however, with the
difference being that whenever a service is deployed/undeployed I end up
replacing the ClassLoader with a new instance that has the service's
ClassLoader added/removed. The replacement is necessary so that unused/old
classes can be reclaimed by the garbage collector and that new versions can
be deployed in the future.

Overall I think this is a more comprehensive approach since it also allows
execution constructs like CacheEntryProcessor implementations provided by
the service to be used across the grid without enabling p2p classloading
(assuming the service is deployed to every node).

There are a few concerns/issues with this approach:

1) There are still isolation concerns with class versions across different
services, but as long as the services don't have dependencies between each
other or have overlapping class usage in a cache or execution context then
it's a non-issue.
2) Using OnheapCacheEnabled in conjunction with service provided domain
classes is impossible since once the service is reloaded and we get a new
classloader all calls to deserialize will fail.
3) This approach only works if nothing caches the result of
IgniteConfiguration.getClassLoader, which isn't obvious since the docs
related to classloader behavior in Ignite is pretty sparse (or at least I
could not find it). I don't think anything does when I check that method
usage, but I'm not 100% sure about that.



--
View this message in context: 
http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p18358.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[npordash]

Thanks Denis!



--
View this message in context: 
http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17524.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[npordash]

Hey Denis,

I tried your solution as that is what Alexei was more-or-less suggesting:
create a delegating classloader, but in this case delegate to whatever
context class loader is set. The problem is that resolved classes will
always be stored by the instantiated classloader, which would be the
delegating one here, and there is no way around that from what I can tell.
There is a fair amount of native code involved and I'm not sure exactly
what's going on there (f.e. Class.forName eventually calls Class.forName0
which is native and ClassLoader.loadClass eventually calls
ClassLoader.resolveClass0 which is also native). 

I ran a test to prove out the above where I created a dynamic class using
ByteBuddy and the following happened:
1) Create URLClassLoader and successfully loaded the class there
2) Attempted to resolve the class using the system classloader (failed as
expected)
3) Created a delegating classloader and attempted to resolve (failed as
expected)
4) Set the context classloader to the urlclassloader and attempted to
resolve (worked as expected)
5) Restored the context classloader to the system classloader and attempted
to resolve (worked which I was hoping it didn't do).

Regarding your code snippet - I did see that and was wondering if it could
do something like the following instead:



I believe getContextClassLoader by default doesn't return null anymore
(since after Java 1.4 or something I believe?) but it is still technically
possible.

I think at least when running sql queries or getting data out of a cache it
is probably a safe assumption that if the calling thread wants to
deserialize as a class called Foo then that class is probably already
available via the context classloader and that takes the burden off of
Ignite on trying to "do the right thing" about class resolution.

-Nick



--
View this message in context: 
http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17339.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Denis Magda]

Nick,

Internally, binary marshaller supports as many class loaders as needed. Even 
more, it can store different versions of the same class loaded by different 
loaders. It’s a responsibility of an Ignite component to pass a valid class 
loader to it at deserialization time. 

The cache component simply passes IgniteConfiguration.getClassLoader() all the 
times. If that class loader is not, meaning the marshaller will received ’null’ 
as a class loader value then it will use the system class loader.

Here is on of the places in the caching internals that triggers an object 
deserialization

private BinaryReaderExImpl reader(@Nullable BinaryReaderHandles rCtx, boolean 
forUnmarshal) {
BinaryOffheapInputStream stream = new BinaryOffheapInputStream(ptr, size, 
false);

stream.position(start);

return new BinaryReaderExImpl(ctx,
stream,
ctx.configuration().getClassLoader(),
rCtx,
forUnmarshal);

As an idea, what if you create your custom class loader that will offload to a 
context class loader whenever is needed. Do you think it will work for you?

—
Denis
 
> On Apr 25, 2017, at 4:23 PM, npordash  wrote:
> 
> Hi Alexei,
> 
> I believe the problem with that approach is that I won't be able to reload
> classes because there is a single classloader that Ignite is referencing.
> When I undeploy a service I also want to allow the corresponding classloader
> (there is 1 per service) and classes to get reclaimed by the JVM (if it
> choses to do so, of course).
> 
> It would also prevent multiple versions of the same class from being used.
> If I have two services deployed then I don't want classes from service 1 to
> get resolved by service 2 simply because service 1 was there first and a
> class with the same name was already resolved by the delegating classloader.
> It's entirely possible the version of the class files are different and this
> would lead to some interesting and confusing errors. 
> 
> This is why I was suggesting to refer to the thread context classloader
> during deserialization because it doesn't impact your API and it addresses
> all of the above issues. However, I can't get a feel for the behavioral
> impact of this change by simply looking at the Ignite codebase and I'm not
> sure what the rationale was to restrict classloading to Ignite's
> classloader.
> 
> -Nick
> 
> 
> 
> 
> 
> --
> View this message in context: 
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17224.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Alexei Scherbakov]

Nick,

Have you tried implementing delegating classloader and set it using
IgniteConfiguration.setClassLoader() ?

This should solve your problem without any API changes on Ignite's side.

2017-04-25 22:42 GMT+03:00 npordash :

> Hi Denis, Val,
>
> Please refer to my initial inquiry on the user forum for full context as I
> think that was lost while cross-posting to here:
> http://apache-ignite-users.70518.x6.nabble.com/BinaryObjectImpl-
> deserializeValue-with-specific-ClassLoader-td12055.html
>
> The setting in IgniteConfiguration does not help in this use-case because
> Ignite is started before user-code is dynamically loaded and ran. I also
> can't set it after it has been started because Ignite is a shared resource
> in the JVM and there would be multiple code paths trying to access it that
> resolved from different classloaders.
>
> I think the existing behavior is inconsistent and surprising to the user,
> but I do think this was just an oversight as not a lot of folks in general
> try to dynamically load and run code at runtime. Within the same JVM I can
> write objects to a cache for classes that Ignite can't resolve from it's
> own
> classpath (because it doesn't need to for writes), but if I try to read
> them
> back from the cache (even in the same JVM) I get a ClassNotFoundException.
>
> -Nick
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-
> deserializeValue-with-specific-ClassLoader-tp17126p17221.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>



-- 

Best regards,
Alexei Scherbakov

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[npordash]

Hi Denis, Val,

Please refer to my initial inquiry on the user forum for full context as I
think that was lost while cross-posting to here:
http://apache-ignite-users.70518.x6.nabble.com/BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td12055.html

The setting in IgniteConfiguration does not help in this use-case because
Ignite is started before user-code is dynamically loaded and ran. I also
can't set it after it has been started because Ignite is a shared resource
in the JVM and there would be multiple code paths trying to access it that
resolved from different classloaders.

I think the existing behavior is inconsistent and surprising to the user,
but I do think this was just an oversight as not a lot of folks in general
try to dynamically load and run code at runtime. Within the same JVM I can
write objects to a cache for classes that Ignite can't resolve from it's own
classpath (because it doesn't need to for writes), but if I try to read them
back from the cache (even in the same JVM) I get a ClassNotFoundException.

-Nick



--
View this message in context: 
http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17221.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Denis Magda]

It’s a good point. Thanks for reminding Val.

Nick, please check that the suggested method works for your use case.

—
Denis

> On Apr 25, 2017, at 11:16 AM, Valentin Kulichenko 
>  wrote:
> 
> We allow to provide custom class loader via
> IgniteConfiguration.setClassLoader. If this class loader is ignored during
> deserialization of cache objects, then I believe it's a bug.
> 
> -Val
> 
> On Tue, Apr 25, 2017 at 7:36 PM, Denis Magda  wrote:
> 
>> Nick,
>> 
>> This deserialization issue is related to cache objects. You will get the
>> same kind of exception if you try to deserialize a cache entry inside of a
>> compute task which class was preloaded using the peer-class-loading feature.
>> 
>> Frankly, this is not treated as an issue on Ignite side. We designed cache
>> objects serialization/deserialization in a way it works now - the class has
>> to be in the local classpath.
>> 
>> However, probably it makes sense to rethink this. *Val*, *Vovan*, what are
>> your thoughts on this?
>> 
>> —
>> Denis
>> 
>>> On Apr 24, 2017, at 6:06 PM, npordash  wrote:
>>> 
>>> Hi Denis,
>>> 
> if you want to deserialize an entry then most likely you’re doing this
>> on
> the app side that already has the class in the class path
>>> 
>>> In this particular case the app is a deployed service and the hosting
>> node
>>> doesn't have the class files on its classpath. I implemented a way to
>> deploy
>>> and run services on the grid even if the class files are not known by
>> ignite
>>> (which is current service grid limitation). It works fine except for this
>>> deserialization issue.
>>> 
>>> -Nick
>>> 
>>> 
>>> 
>>> --
>>> View this message in context: http://apache-ignite-
>> developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-
>> deserializeValue-with-specific-ClassLoader-tp17126p17173.html
>>> Sent from the Apache Ignite Developers mailing list archive at
>> Nabble.com.
>> 
>> 

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Valentin Kulichenko]

We allow to provide custom class loader via
IgniteConfiguration.setClassLoader. If this class loader is ignored during
deserialization of cache objects, then I believe it's a bug.

-Val

On Tue, Apr 25, 2017 at 7:36 PM, Denis Magda  wrote:

> Nick,
>
> This deserialization issue is related to cache objects. You will get the
> same kind of exception if you try to deserialize a cache entry inside of a
> compute task which class was preloaded using the peer-class-loading feature.
>
> Frankly, this is not treated as an issue on Ignite side. We designed cache
> objects serialization/deserialization in a way it works now - the class has
> to be in the local classpath.
>
> However, probably it makes sense to rethink this. *Val*, *Vovan*, what are
> your thoughts on this?
>
> —
> Denis
>
> > On Apr 24, 2017, at 6:06 PM, npordash  wrote:
> >
> > Hi Denis,
> >
> >>> if you want to deserialize an entry then most likely you’re doing this
> on
> >>> the app side that already has the class in the class path
> >
> > In this particular case the app is a deployed service and the hosting
> node
> > doesn't have the class files on its classpath. I implemented a way to
> deploy
> > and run services on the grid even if the class files are not known by
> ignite
> > (which is current service grid limitation). It works fine except for this
> > deserialization issue.
> >
> > -Nick
> >
> >
> >
> > --
> > View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-
> deserializeValue-with-specific-ClassLoader-tp17126p17173.html
> > Sent from the Apache Ignite Developers mailing list archive at
> Nabble.com.
>
>

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Denis Magda]

Nick,

This deserialization issue is related to cache objects. You will get the same 
kind of exception if you try to deserialize a cache entry inside of a compute 
task which class was preloaded using the peer-class-loading feature.

Frankly, this is not treated as an issue on Ignite side. We designed cache 
objects serialization/deserialization in a way it works now - the class has to 
be in the local classpath.

However, probably it makes sense to rethink this. *Val*, *Vovan*, what are your 
thoughts on this?

—
Denis

> On Apr 24, 2017, at 6:06 PM, npordash  wrote:
> 
> Hi Denis,
> 
>>> if you want to deserialize an entry then most likely you’re doing this on
>>> the app side that already has the class in the class path
> 
> In this particular case the app is a deployed service and the hosting node
> doesn't have the class files on its classpath. I implemented a way to deploy
> and run services on the grid even if the class files are not known by ignite
> (which is current service grid limitation). It works fine except for this
> deserialization issue.
> 
> -Nick
> 
> 
> 
> --
> View this message in context: 
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17173.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[npordash]

Hi Denis,

>> if you want to deserialize an entry then most likely you’re doing this on
>> the app side that already has the class in the class path

In this particular case the app is a deployed service and the hosting node
doesn't have the class files on its classpath. I implemented a way to deploy
and run services on the grid even if the class files are not known by ignite
(which is current service grid limitation). It works fine except for this
deserialization issue.

-Nick



--
View this message in context: 
http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-tp17126p17173.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Denis Magda]

Hi guys,


Cache entries don’t store an identification of the class loader a key or value 
was created with. This is why binary marshaller picks the system class loader 
at deserialization time by default and you get class not found exception.

>> In general, I think ignite should try to resolve a class based on the 
>> caller's context first instead of only relying on
>> ignite's classloader or what it was configured with.

That’s probably worth looking into but for the cache entries we tried to 
simplify the things - if you want to deserialize an entry then most likely 
you’re doing this on the app side that already has the class in the class path. 
For the server nodes, that usually doesn’t have the classes it’s suggest to use 
BinaryObject and BinaryObjectBuilder wrappers on top of serialized data:
https://apacheignite.readme.io/docs/binary-marshaller#binaryobject-cache-api

Vovan, what do you think on this?

—
Denis

> On Apr 24, 2017, at 12:55 AM, Dmitriy Karachentsev 
>  wrote:
> 
> Crosspost to dev list.
> 
> Igniters,
> 
> This proposal looks quite reasonable. Do we have any restrictions that
> could prevent implementing such feature?
> I think it's nice to have in Ignite.
> 
> Thanks!
> -Dmitry.
> 
> On Sun, Apr 23, 2017 at 1:37 AM, npordash  wrote:
> 
>> Thanks!
>> 
>> That would definitely help address the hack I've implemented where I have
>> to
>> reference classes in Ignite's internal package.
>> 
>> However, I still have to work with the caches in binary which is less than
>> ideal. It's pretty common in use-cases like this to first try to use
>> Thread.currentThread().getContextClassLoader() and if that's not set then
>> fallback to something else. In general, I think ignite should try to
>> resolve
>> a class based on the caller's context first instead of only relying on
>> ignite's classloader or what it was configured with.
>> 
>> In the spirit of the webinar that Denis just had, I think this kind of
>> behavior will become mandatory for the service grid to get to the point
>> where it can do deployments of services without requiring class files to be
>> on ignite's classpath. I've heard that is something that's still
>> tentatively
>> planned and the use-case I outlined is an attempt to get around the current
>> service grid limitations. :)
>> 
>> WDYT?
>> 
>> -Nick
>> 
>> 
>> 
>> --
>> View this message in context: http://apache-ignite-users.
>> 70518.x6.nabble.com/BinaryObjectImpl-deserializeValue-with-
>> specific-ClassLoader-tp12055p12171.html
>> Sent from the Apache Ignite Users mailing list archive at Nabble.com.
>>