2011/5/19 Galder Zamarreño <gal...@redhat.com>: > > On May 19, 2011, at 10:04 AM, Sanne Grinovero wrote: > >> 2011/5/19 Dan Berindei <dan.berin...@gmail.com>: >>> On Wed, May 18, 2011 at 7:06 PM, Manik Surtani <ma...@jboss.org> wrote: >>>> Hi guys >>>> >>>> Sorry I've been absent from this thread for a while now (it's been growing >>>> faster than I've been able to deal with email backlog!) >>>> >>>> Anyway, this is a very interesting discussion. To summarise - as Pete did >>>> at some point - there are 2 goals here: >>>> >>>> 1. Safe and intuitive use of an appropriate classloader >>>> 2. Safe type system for return values. >>>> >>>> I think the far more pressing concern is (1) so I'd like to focus on that. >>>> If we think (2) is pressing enough a concern, we should spawn a separate >>>> thread and discuss there. >>>> >>>> So, onto the issue of safe classloading. >>>> >>>> 1) Class loader per session/cache. >>>> >>>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and >>>> specifically I think this is best achieved as a delegate to a cache, again >>>> as suggested elsewhere by Pete, etc. E.g., >>>> >>>> Cache<?, ?> myCache = cacheManager.getCache("myCache", >>>> myClassLoader); >>>> >>>> and what is returned is something that delegates to the actual cache, >>>> making sure the TCCL is set and re-set appropriately. The handle to the >>>> cache is effectively your "session" and each webapp, etc in an EE >>>> environment will have its own handle. I propose using the TCCL as an >>>> internal implementation detail within this delegate, helps with making >>>> sure it is carefully managed and cleaned up while not re-engineering loads >>>> of internals. >>>> >>> >>> I like the API but I would not recommend using the TCCL for this. I >>> was able to get a nice perf jump in the HotRod client by skipping 2 >>> Thread.setContextClassLoader() calls on each cache operation (1 to set >>> the TCCL we wanted and 1 to restore the original TCCL). Setting the >>> TCCL is a privileged operation, so it has to go through a >>> SecurityManager and that is very slow. >>> >>> >>>> I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is >>>> enough ... it is clear enough, and I don't see the need for overloaded >>>> getCache(name, classOfWhichClassLoaderIWishToUse). >>>> >>> >>> I agree, a Cache.usingClassloader(classOfWhichClassLoaderIWishToUse) >>> overload would have made sense because the method name already >>> communicates the intention, but a getCache(name, clazz) overload is >>> too obscure. >>> >>> >>>> 2) Class loader per invocation. >>>> >>>> I've been less than happy with this, not just because it pollutes the API >>>> but that it adds a layer of confusion. If all use cases discussed can be >>>> solved with (1) above, then I'd prefer to just do that. >>>> >>>> The way I see it, most user apps directly using Infinispan would only be >>>> exposed to a single class loader per cache reference (even if multiple >>>> references talk to the same cache). >>>> >>>> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on >>>> this thread. So this is a question for Galder - is it feasible to >>>> maintain several references to a cache, one for each app/persistence unit? >>>> >>>> 3) Can all OSGi requirements be handled by (1)? I would guess so, from >>>> what I have read here, since the class loader is explicitly passed in when >>>> getting a handle on a cache. >>>> >>> >>> Yes, the only difference is that OSGi goesn't make any guarantees >>> about the TCCL, so passing the classloader explicitly will work in all >>> environments. However, >>> >>> 4) What about storeAsBinary="false"? Threads processing requests from >>> other nodes are not associated with any CacheManager.getCache(name, >>> classloader) call, and they also have to unmarshall values with this >>> setting. >>> >>> Since Hibernate already mandates storeAsBinary="true" for its 2LC, we >>> can probably get away with only supporting one classloader per cache >>> in the storeAsBinary="false" case. >>> >>> Still, we can't rely on the TCCL of the background threads because >>> those threads are shared between all the caches in a CacheManager. In >>> fact we should probably set the TCCL to null for all background >>> threads created by Infinispan, or we risk keeping the classes of the >>> first application/bundle that called us alive as long as those threads >>> are still running. >>> >>> Dan >> >> Totally agree with all you said, great analysis! >> >> So it looks like that all Caches being shared across different >> classloaders (deployed applications) >> should be used only with storeAsBinary="false", still I'm having some >> doubts about how even that is safe. >> How can we prevent two applications both using hibernate from mixing >> the cache keys and values? > > How do you do that? Apps don't have any direct access to the Hibernate Cache. > >> I assume the keys being used in the 2LC use entity names or class >> names combined with the primary keys; >> it's totally possible to have multiple applications using the same >> entity names, which is quite common in my experience. > > That's what region name comes in. Multiple apps shouldn't have the same > region name. That's what discriminates entities in different apps.
ah, thank you I finally understand why the configuration property hibernate.cache.region_prefix can be very useful. Actually, it would be nice to document this explicitly as otherwise people might end up deserializing a binary stream in the wrong application.. Sanne > >> >> The configurations defined in the app server could be used as >> templates to start/stop specific caches for each deployment, >> generating a unique name depending on the ear/war/.. in such a way to >> be consistent with names on other nodes. >> Was this all designed as a workaround for the fact that we're still >> having trouble in forming a cluster of cachemanagers having different >> caches? >> >> Sanne >> >>> >>> >>>> Cheers >>>> Manik >>>> >>>> >>>> On 27 Apr 2011, at 17:39, Jason T. Greene wrote: >>>> >>>>> Available here: >>>>> https://github.com/infinispan/infinispan/pull/278 >>>>> >>>>> The problem is basically that infinispan currently is using TCCL for all >>>>> class loading and resource loading. This has a lot of problems in >>>>> modular containers (OSGi, AS7, etc), where you dont have framework >>>>> implementation classes on the same classloader as user classes (that is >>>>> how they achieve true isolation) >>>>> >>>>> You can read about this in more detail here: >>>>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide >>>>> >>>>> The patch in the pull request is a first step, and should fix many >>>>> issues, but it does not address all (there is still a lot of TCCL usage >>>>> spread out among cacheloaders and so on), and ultimately it's just a >>>>> work around. It should, however, be compatible in any other non-modular >>>>> environment. >>>>> >>>>> Really the ultimate solution is to setup a proper demarcation between >>>>> what the user is supposed to provide, and what is expected to be bundled >>>>> with infinispan. Whenever there is something the user can provide a >>>>> class, then the API should accept a classloader to load that class from. >>>>> If it's a class that is just internal wiring of infinispan, then >>>>> Infinispan's defining classloader should always be used. >>>>> >>>>> The one case I can think of in infnispan where TCCL really makes sense >>>>> is in the case of lazy deserialization from an EE application. However >>>>> that is only guaranteed to work when you are executing in a context that >>>>> has that style of contract (like in an EE component). There are many >>>>> other cases though where someone would expect it to work from a non-EE >>>>> context (e.g. from a thread pool). What you really want is the caller's >>>>> classloader, but that is not cheap to get at, so it's something that >>>>> should be supplied as part of the API interaction (in the case where >>>>> there is no EE context). Alternatively you can just just require that >>>>> folks push/pop TCCL, but users often get this wrong. >>>>> >>>>> Thanks! >>>>> >>>>> -- >>>>> Jason T. Greene >>>>> JBoss, a division of Red Hat >>>>> _______________________________________________ >>>>> infinispan-dev mailing list >>>>> infinispan-dev@lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>> >>>> -- >>>> Manik Surtani >>>> ma...@jboss.org >>>> twitter.com/maniksurtani >>>> >>>> Lead, Infinispan >>>> http://www.infinispan.org >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev@lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>> >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev@lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@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@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev