On Thu, Nov 22, 2012 at 11:53 AM, Galder Zamarreño <[email protected]>wrote:
> > On Nov 21, 2012, at 4:49 PM, Mircea Markus <[email protected]> wrote: > > > Hi, > > > > Part of fixing ISPN-2435, I need to significantly change > DistributionInterceptor which at the moment is a very complex pice of code. > Building the fix on top of it is extremely difficult and error prone, so I > need to refactor it a bit before moving forward. > > One such refactoring is about changing the way the async operations are > handled (e.g. putAsync()). At the moment all the interceptor calls happen > in user's thread, but two remote calls which are invoked with futures and > aggregated: > > the L1 invalidation and the actual distribution call. The code for > handling this future aggregation is rather complicated and spreads over > multiple classes (RpcManager, L1Manager, ReplicationInterceptor, > DistributionInterceptor), so the simple alternative solution I have in mind > is to build an asycPut on top of a syncPut and wrap it in a future: > > > > CacheImpl:putAsync(k,v) { > > final InvocationContext ic = > createInvocatinonContextInCallerThread(); //this is for class loading > purpose > > return asyncPoolExecutor.submit(new Callable() { > > public Object call() { > > return put(k,v, ic); //this is the actual sync put > > } > > } > > } > > > > This would significantly simplify several components ( no references to > network/aggregated futures in RpcManager, L1Manager, > ReplicationInterceptor, DistributionInterceptor). > > ^ At first glance, that's how I'd have implemented this feature, but Manik > went down the route of wrapping in futures only those operations that went > remote. > > Maybe he was worried about ctx switch cost? Or maybe about ownership of > locks when these are acquired in a separate thread from the actual caller > thread? > Speaking of locks, does putAsync make sense in a transactional context? There may be another backwards compatibility issue here, with listeners that expect to be called on the caller's thread (e.g. to use the TM transaction that's stored in a thread-local). > > Possible issues: > > - caller's class loader - the class loader is aggregated in the > InvocationContext, so as long as we build the class loader in caller's > thread we should be fine > > ^ To be precise, we don't build a class loaders. I guess you're refering > at building the invocation context. > > These days we're more tight wrt the classloader used, avoiding the > reliance on the TCCL, so I think we're in a safer position. > > > - IsMarshallableInterceptor is used with async marshalling, in order to > notify the user when objects added to the cache are not serializable. With > the approach I suggested, for async calls only (e.g. putAsync) this > notification would not happen in caller's thread, but async on > future.get(). I really don't expect users to rely on this functionality, > but something that would change never the less. > > ^ I don't think this is crucial. You need to call future.get() to find out > if things worked correctly or not, regardless of cause. > > > - anything else you can think of? > > > > I know this is a significant change at this stage in the project, so I > really tried to go without it - but that resulted in spaghetti code taking > a lot of time to patch. So instead of spending that time to code a complex > hack I'd rather go for the simple and nice solution and add more unit tests > to prove it works. > > ^ Have you done some experimenting already? > > Cheers, > > > > > Cheers, > > -- > > Mircea Markus > > Infinispan lead (www.infinispan.org) > > > > > > > > > > _______________________________________________ > > infinispan-dev mailing list > > [email protected] > > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Galder Zamarreño > [email protected] > twitter.com/galderz > > Project Lead, Escalante > http://escalante.io > > Engineer, Infinispan > http://infinispan.org > > > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev >
_______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
