[infinispan-dev] [Pull Request] Modular Classloading Compatibility
Galder Zamarreño
galder at redhat.com
Wed May 18 03:54:45 EDT 2011
On May 17, 2011, at 12:04 AM, Dan Berindei wrote:
> Not to be the guy that breaks the circle, how about the threads that
> handle incoming RPCs from other nodes? With storeAsBinary="false"
> those threads have to unmarshal the objects in order to store them on
> the local node, so we'd need a way to register a classloader with the
> cache, not just with the cache session.
>
> If we use a single cache for all Hibernate entities, regardless of
> application, this means the cache could receive application objects
> from the other nodes without the application even being deployed. Does
> this mean we'll require storeAsBinary="true" in all Hibernate 2LC
> configurations?
storeAsBinary has always been a requirement for Hibernate 2LC and this use case is configured with it.
>
> Dan
>
>
> On Mon, May 16, 2011 at 9:54 PM, David M. Lloyd <david.lloyd at redhat.com> wrote:
>> I know, we can just attach the class loader to the cache!
>>
>> Okay, just kidding, but Galder is right, this conversation is going in
>> circles. We already discussed that in this thread and a number of
>> points were raised for and against.
>>
>> On 05/16/2011 01:20 PM, Adrian Cole wrote:
>>> What about a helper that just returns a cache with a specific
>>> classloader from a cache?
>>>
>>> cache.withLoader(cl).get(K key)
>>>
>>> -a
>>>
>>>
>>> On Mon, May 16, 2011 at 11:14 AM, Galder Zamarreño <galder at redhat.com
>>> <mailto:galder at redhat.com>> wrote:
>>>
>>>
>>> On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:
>>>
>>> > I don't like the TCCL either, so I'll repeat my suggestion from two
>>> > weeks ago to just have:
>>> >
>>> > Cache c = cacheManager.getCache( cacheName, classLoader );
>>> >
>>> > sounds reasonable to me to have the application declare it's
>>> intentions once ?
>>> >
>>> > BTW I don't like
>>> >
>>> > "cache.get(K key, Class<V> clazz)"
>>> >
>>> > as we're not speaking only about the get(K) method, but about many
>>> > methods and this will explode the number of method of Cache; on the
>>> > other hand I think it;s acceptable to have a single Cache instance
>>> > used by a single application/classloader. You can still have multiple
>>> > applications share the same underlying cache and use different
>>> > classloaders:
>>>
>>> Guys, we're going around in circles. As I said the other week, you
>>> can't assume 1 cache = 1 classloader cos for example in the
>>> Hibernate 2LC all entities will be stored in a single cache as
>>> opposed to today where we have a cache per entity. And if all
>>> entities are stored in the same cache, we potentially have a cache
>>> that contains data belonging to multiple cache loaders. And the
>>> reason for all this is cos we don't support asymmetric clusters.
>>>
>>> Could someone start a design wiki to grab all the requirements?
>>>
>>> >
>>> > getCache( cacheName, classLoader ) would return a delegate to the
>>> > original cache, having a specific marshaller in the invocation
>>> context
>>> > as Trustin was suggesting.
>>> >
>>> > Cheers,
>>> > Sanne
>>> >
>>> >
>>> > 2011/5/16 Pete Muir <pmuir at redhat.com <mailto:pmuir at redhat.com>>:
>>> >>
>>> >> On 16 May 2011, at 18:20, Galder Zamarreño wrote:
>>> >>
>>> >>>
>>> >>> On May 12, 2011, at 11:18 AM, Dan Berindei wrote:
>>> >>>
>>> >>>> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert
>>> <david at redhat.com <mailto:david at redhat.com>> wrote:
>>> >>>>> On 11/05/2011 17:54, Dan Berindei wrote:
>>> >>>>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir<pmuir at redhat.com
>>> <mailto:pmuir at redhat.com>> wrote:
>>> >>>>>>> Were we developing for OSGi I would certainly agree with
>>> you. However in many environments today we can reasonably expect the
>>> TCCL to be set and to be able to load the classes we need. So whilst
>>> making it part of the API is the safest option, it's also making
>>> complicated an API for the sake of the few at the cost of the many.
>>> Further this also seems kinda nasty to me. We know the class (and
>>> hence bundle/module) when we put the object into Infinispan,
>>> therefore why do we require people to respecify this again?
>>> >>>>>>>
>>> >>>>>>> David, can we not actually do something here akin to what
>>> we are discussing for Weld? Whereby we can serialize out the bundle
>>> id and then find the correct CL based on that when we deserialize.
>>> >>>>>> What if the object is a java.util.ArrayList? Each element in
>>> the list
>>> >>>>>> could belong to a different bundle, so you'd have to write a
>>> bundle id
>>> >>>>>> for every element in the list.
>>> >>>>> Yes, if you know the Bundle-SymbolicName and Version (or the
>>> Bundle ID)
>>> >>>>> you can find its classloader.
>>> >>>>>
>>> >>>>> On the other question, if you're passing in a class object
>>> then you can
>>> >>>>> obtain its classloader and hence the bundle where it came
>>> from. But, and
>>> >>>>> I think this is what Dan allused to above, is it always true
>>> that the
>>> >>>>> class your passing in comes from the bundle that you need to
>>> have or
>>> >>>>> could it also come from one of its parent class loaders?
>>> >>>>>
>>> >>>>
>>> >>>> Exactly David, sorry if my message was a little cryptic. I
>>> think in
>>> >>>> order to handle every case properly you would have to go
>>> through the
>>> >>>> entire object graph being stored in the cache in order to find
>>> all the
>>> >>>> classloaders/bundle ids that you will need on get().
>>> >>>>
>>> >>>> That seems like a lot of overhead to me, and forcing the user to
>>> >>>> provide the classloader doesn't seem that bad in comparison.
>>> Perhaps
>>> >>>> we should use something other than a thread-local for this
>>> though, so
>>> >>>> that users can do a onto the result of a
>>> >>>> cacheManager.getCache("A").usingClassLoader(A.class) and never
>>> have to
>>> >>>> provide the classloader again.
>>> >>>>
>>> >>>> In fact I think this is a good idea for the invocation flags we
>>> >>>> already have, too. It would involve creating lots of overloads in
>>> >>>> CacheDelegate with a PreInvocationContext parameter and a new
>>> >>>> CacheDelegateWithContext class to invoke those methods, but
>>> the public
>>> >>>> API would remain the same.
>>> >>>
>>> >>> No matter how I look at it, putting a classloader in a thread
>>> local makes me shiver.
>>> >>
>>> >> I also wonder why we want do this, given we already have a
>>> construct called the Thread Local Context Classloader ;-)
>>> >>
>>> >> Either we use that, or use some other mechanism.
>>> >>
>>> >>> Just imagine the mayhem you can cause if you "forget" to clear
>>> the thread local.
>>> >>>
>>> >>> I've done enough of Apache Commons Logging support to
>>> understand that you should limit the references to classloaders to
>>> the minimum, particularly in system classes/infrastructure.
>>> >>>
>>> >>> If we need to end up forcing users to register classloaders in
>>> these scenarions, we need to do it in such way that either:
>>> >>>
>>> >>> - we can detect these leaks (it might be a bit primitive now
>>> but old JBoss JCA code had an interesting way of discovering
>>> unclosed open connections)
>>> >>>
>>> >>> - if we give you on trying to detect them, the impact of a leak
>>> is reduced as much as possible.
>>> >>>
>>> >>> Cheers,
>>> >>> --
>>> >>> Galder Zamarreño
>>> >>> Sr. Software Engineer
>>> >>> Infinispan, JBoss Cache
>>> >>>
>>> >>>
>>> >>> _______________________________________________
>>> >>> infinispan-dev mailing list
>>> >>> infinispan-dev at lists.jboss.org
>>> <mailto:infinispan-dev at lists.jboss.org>
>>> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>> >>
>>> >>
>>> >> _______________________________________________
>>> >> infinispan-dev mailing list
>>> >> infinispan-dev at lists.jboss.org
>>> <mailto:infinispan-dev at lists.jboss.org>
>>> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>> >>
>>> >
>>> > _______________________________________________
>>> > infinispan-dev mailing list
>>> > infinispan-dev at lists.jboss.org
>>> <mailto:infinispan-dev at lists.jboss.org>
>>> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>> --
>>> Galder Zamarreño
>>> Sr. Software Engineer
>>> Infinispan, JBoss Cache
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org <mailto:infinispan-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>>
>> --
>> - DML
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
More information about the infinispan-dev
mailing list