<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On 23 Nov 2012, at 09:35, Galder Zamarreño wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><br>On Nov 22, 2012, at 3:03 PM, Dan Berindei &lt;<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>&gt; wrote:<br><br><blockquote type="cite">On Thu, Nov 22, 2012 at 3:31 PM, Mircea Markus &lt;<a href="mailto:mmarkus@redhat.com">mmarkus@redhat.com</a>&gt; wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">On 22 Nov 2012, at 10:16, Dan Berindei wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">On Thu, Nov 22, 2012 at 11:53 AM, Galder Zamarreño &lt;<a href="mailto:galder@redhat.com">galder@redhat.com</a>&gt; wrote:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">On Nov 21, 2012, at 4:49 PM, Mircea Markus &lt;<a href="mailto:mmarkus@redhat.com">mmarkus@redhat.com</a>&gt; wrote:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Hi,<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">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.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">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:<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">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:<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">CacheImpl:putAsync(k,v) {<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;final InvocationContext ic = createInvocatinonContextInCallerThread(); //this is for class loading purpose<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;return asyncPoolExecutor.submit(new Callable() {<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;public Object call() {<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return put(k,v, ic); //this is the actual sync put<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;}<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> &nbsp;&nbsp;&nbsp;}<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">}<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">This would significantly simplify several components ( no references to network/aggregated futures in RpcManager, L1Manager, ReplicationInterceptor, DistributionInterceptor).<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">^ 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.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">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?<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Speaking of locks, does putAsync make sense in a transactional context?<br></blockquote></blockquote><blockquote type="cite">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. <br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">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.<br></blockquote><br>^ I think this is important. Even if you call putAsync(), it should participate in any ongoing transactions without any problems.<br></div></blockquote>yes, &nbsp;that behaviour should be indeed preserved.<br><blockquote type="cite"><div><br>@Mircea, what I mean earlier is whether you had prototyped your current suggestion to have putAsync() submit a callable…etc.<br><br>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.<br></div></blockquote></div>You are right, I'm very closed to finish the implementation based on this refactoring&nbsp;:-)&nbsp;The main issues I had were around asyn ops + transactions, the point Dan raised.<div><br><div apple-content-edited="true">
<span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Cheers,</div><div>--&nbsp;<br>Mircea Markus</div><div>Infinispan lead&nbsp;(<a href="http://www.infinispan.org">www.infinispan.org</a>)</div><div><br></div></div></span></div></span></div></span></div></span><br class="Apple-interchange-newline"></span><br class="Apple-interchange-newline">
</div>
<br></div></body></html>