On 23 Nov 2012, at 09:35, Galder Zamarreño wrote: > > On Nov 22, 2012, at 3:03 PM, Dan Berindei <[email protected]> wrote: > >> On Thu, Nov 22, 2012 at 3:31 PM, Mircea Markus <[email protected]> wrote: >> >> On 22 Nov 2012, at 10:16, Dan Berindei wrote: >> >>> >>> 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? >> Good point, I don't think async operation should work in the context of >> transaction: that would mean having two threads(the async operation thread >> and the 'main' thread) working on the same javax.transaction.Transaction >> object concurrently which is something not supported by most TM afaik, and >> something we don't support internally. >> >> >> I'm not sure, but I think it is supported now - the only things happening on >> a different thread only care about the cache's transaction, and not about >> the TM transaction. > > ^ I think this is important. Even if you call putAsync(), it should > participate in any ongoing transactions without any problems. yes, that behaviour should be indeed preserved. > > @Mircea, what I mean earlier is whether you had prototyped your current > suggestion to have putAsync() submit a callable…etc. > > The reason I ask is cos I don't think it should take you very long to > prototype this new way of dealing with async methods, and by running the > testsuite you might encounter other issues, apart from the one implied here > wrt transactions. You are right, I'm very closed to finish the implementation based on this refactoring :-) The main issues I had were around asyn ops + transactions, the point Dan raised.
Cheers, -- Mircea Markus Infinispan lead (www.infinispan.org)
_______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
