Re: BinaryObjectImpl.deserializeValue with specific ClassLoader

[Denis Magda <dm...@apache.org>]

Nick,

Looks like we need to look into it, thanks for extensive explanation and your time. I’ve updated the originally created ticket:
https://issues.apache.org/jira/browse/IGNITE-5038

Feel free to expand the description if needed.

—
Denis

> On Apr 28, 2017, at 4:35 PM, npordash <ni...@gmail.com> wrote:
> 
> 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

[Valentin Kulichenko <va...@gmail.com>]

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 <ni...@gmail.com> 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 <ni...@gmail.com>]

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 <ni...@gmail.com>]

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.