Glad you already figured it out.
Removing generic is for purpose, since AsyncCallbackDispatcher sits at the
bottom of the chain that needs to deal with different kinds of callback
completion routines. Type-safe check for callback completion routine is
done through AsyncCompletionCall<T> itself
public class AsyncCallbackDispatcher<T, R> implements
AsyncCompletionCallback
Kelven
On 1/28/14, 11:16 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com>
wrote:
>Cancel that query. :)
>
>It looks like public class AsyncCallbackDispatcher<T, R> implements
>AsyncCompletionCallback<R>
>breaks a couple places in the codebase.
>
>
>On Tue, Jan 28, 2014 at 11:56 PM, Mike Tutkowski <
>mike.tutkow...@solidfire.com> wrote:
>
>> Hi Kelven,
>>
>> Thanks for the info.
>>
>> Yeah, I figured it "appeared" to be over engineered in this regard
>>because
>> there probably was (and possibly still is) a grander vision of where
>>we'd
>> like to take CloudStack.
>>
>> You know, on a related note, I noticed the following:
>>
>> public class AsyncCallbackDispatcher<T, R>
>>implementsAsyncCompletionCallback
>>
>> I changed it to this in my local repo:
>>
>> public class AsyncCallbackDispatcher<T, R>
>>implementsAsyncCompletionCallback<R>
>>
>> That makes sense to me. Do you see any issue with that change?
>>
>> Thanks again!
>>
>>
>> On Tue, Jan 28, 2014 at 11:32 PM, Kelven Yang
>><kelven.y...@citrix.com>wrote:
>>
>>> Originally we did think to introduce async model to CloudStack
>>> orchestration engine, using queues, event notifications etc to make the
>>> system more loosely-coupled and use much less thread resources with
>>>async
>>> programming. However, due the large code base we already have and
>>> synchronous programming is everywhere, it end up to where we are now.
>>>
>>> Async programing is also not async-method based programming, you may
>>>view
>>> it as over-engineering, it is truly un-completed engineering in my
>>> opinion. The same goes to the VMSync refactoring as well, job
>>>framework is
>>> refactored to support putting a job on hold and waking it up upon
>>>events
>>> but in reality, you will still see that we just hold a thread waiting
>>>for
>>> completion. So we definitely still have a lot of work to do for
>>>CloudStack
>>> to continuously evolve itself
>>>
>>> Kelven
>>>
>>> On 1/28/14, 4:52 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com>
>>> wrote:
>>>
>>> >It seems in the cases that I've observed this pattern that we don't
>>> >actually do anything asynchronously. We call a method that has the
>>>word
>>> >"Async" in it typically, but this method does everything in sequence
>>>and
>>> >then returns a "future" object. The calling code then calls get() on
>>>the
>>> >future object, which returns immediately because everything was
>>>executed
>>> >in
>>> >sequence in the "Async" method.
>>> >
>>> >I was curious, were we planning on making many of these operations
>>>truly
>>> >asynchronous in the future?
>>> >
>>> >If not, I wonder if there's a bit of over-engineering going on here?
>>>If
>>> >they are always going to be done synchronously, we could just call the
>>> >"callback" method after calling the "async" method.
>>> >
>>> >Thoughts on this?
>>> >
>>> >Thanks :)
>>> >
>>> >
>>> >On Tue, Jan 28, 2014 at 3:42 PM, Kelven Yang <kelven.y...@citrix.com>
>>> >wrote:
>>> >
>>> >> I originally thought to just ask developer to use plain string
>>> literals,
>>> >> it has better readability but it can't take advantage of IDE
>>>provided
>>> >> refactoring (callback method renames), it ended up to this crazy
>>> >>approach.
>>> >> Hopefully one day Java may provide C# delegate like feature, then
>>>all
>>> >> these hacking tricks can go away
>>> >>
>>> >> Kelven
>>> >>
>>> >> On 1/28/14, 2:29 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com>
>>> >> wrote:
>>> >>
>>> >> >I see...just for chaining purposes. In some places I notice we
>>>chain
>>> >>this
>>> >> >and in others we don't.
>>> >> >
>>> >> >Thanks for the clarification!
>>> >> >
>>> >> >
>>> >> >On Tue, Jan 28, 2014 at 3:13 PM, Kelven Yang
>>><kelven.y...@citrix.com>
>>> >> >wrote:
>>> >> >
>>> >> >> This is to support fluent coding style to chain all the callback
>>> >>setup
>>> >> >> calls in one line. Using caller.getTarget().callbackMethod()
>>>alone
>>> >>may
>>> >> >> cause people to think the statement as a logic requirement and
>>> >>actually
>>> >> >>it
>>> >> >> is not but hackings.
>>> >> >>
>>> >> >> It is like to setup a function pointer in C or delegate in C#,
>>>but
>>> >>it is
>>> >> >> kind of alien in Java world since Java does not have language
>>>level
>>> >> >> support for this yet.
>>> >> >>
>>> >> >> Kelven
>>> >> >>
>>> >> >> On 1/28/14, 11:06 AM, "Mike Tutkowski" <
>>> mike.tutkow...@solidfire.com>
>>> >> >> wrote:
>>> >> >>
>>> >> >> >Thanks Kelven
>>> >> >> >
>>> >> >> >Yeah, that code is pretty crazy. :)
>>> >> >> >
>>> >> >> >I followed that the getTarget() method actually dynamically
>>>extends
>>> >>a
>>> >> >> >class
>>> >> >> >and allows us to inject logic in the new class that enables us
>>>to
>>> >>save
>>> >> >> >createVolumeFromBaseImageCallBack as the method we want to
>>>invoke
>>> >>when
>>> >> >>our
>>> >> >> >async operation has completed.
>>> >> >> >
>>> >> >> >It does provide the benefit that you don't have to pass in a
>>>string
>>> >> >>that
>>> >> >> >represents the method name.
>>> >> >> >
>>> >> >> >What I was actually wondering about, though, is why we surround
>>> >> >> >caller.getTarget().createVolumeFromBaseImageCallBack(null,
>>>null);
>>> >>with
>>> >> >> >caller.setCallback();?
>>> >> >> >
>>> >> >> >It seems that setCallback() ignores the input parameter and just
>>> >> >>returns
>>> >> >> >"this";
>>> >> >> >
>>> >> >> >Wouldn't
>>>caller.getTarget().createVolumeFromBaseImageCallBack(null,
>>> >> >>null);
>>> >> >> >by itself work exactly the same way?
>>> >> >> >
>>> >> >> >
>>> >> >> >On Tue, Jan 28, 2014 at 11:57 AM, Kelven Yang
>>> >> >> ><kelven.y...@citrix.com>wrote:
>>> >> >> >
>>> >> >> >> Mike,
>>> >> >> >>
>>> >> >> >> This is a very dirty hack that I personally hate it. This is
>>>the
>>> >> >>hack to
>>> >> >> >> utilize Eclipse¹s (or other smart IDE) to do auto-completion
>>>for
>>> >>you
>>> >> >>to
>>> >> >> >> find the right callback method.
>>> >> >> >>
>>> >> >> >> if you write
>>> >> >> >>
>>> >> >> >> caller.getTarget().createVolumeFromBaseImageCallBack(null,
>>> >>null);
>>> >> >> >>
>>> >> >> >> Java will interprete it as a standalone method call,
>>> >> >> >>
>>> >> >> >>
>>> >> >> >> If you write as below, it tries to tell that this is to setup
>>>a
>>> >> >> >>callback,
>>> >> >> >> to return this in caller.setCallback is to let you continue to
>>> use
>>> >> >> >>fluent
>>> >> >> >> style to callback setups
>>> >> >> >>
>>> >> >> >> caller.setCallback(
>>> >> >> >> caller.getTarget().createVolumeFromBaseImageCallBack(null,
>>>null)
>>> >>);,
>>> >> >> >>
>>> >> >> >> Behind scene, it uses CGLIB for dispatcher to figure out which
>>> >> >>method is
>>> >> >> >> the callback without requiring developer to give it as literal
>>> >>string
>>> >> >> >>
>>> >> >> >>
>>> >> >> >> AsyncMethod pattern is used commonly in parallel algorithms to
>>> >> >> >>dynamically
>>> >> >> >> branch out sub-calculations, I think it does not fit well in
>>> >> >>CloudStack,
>>> >> >> >> and also due to the lack of language feature in Java, this
>>> >>hacking
>>> >> >> >> technique makes the code really hard to read
>>> >> >> >>
>>> >> >> >> Kelven
>>> >> >> >>
>>> >> >> >> On 1/27/14, 6:55 PM, "Mike Tutkowski"
>>> >><mike.tutkow...@solidfire.com>
>>> >> >> >> wrote:
>>> >> >> >>
>>> >> >> >> >Hi,
>>> >> >> >> >
>>> >> >> >> >I've been looking at our callback pattern.
>>> >> >> >> >
>>> >> >> >> >Can someone explain why we always seem to do this?:
>>> >> >> >> >
>>> >> >> >>
>>> >> >>
>>> >>
>>>
>>>
>>>>>>>>>>caller.setCallback(caller.getTarget().createVolumeFromBaseImageCa
>>>>>>>>>>llB
>>> >>>>>>>ac
>>> >> >>>>>k(
>>> >> >> >>>nu
>>> >> >> >> >ll,
>>> >> >> >> >null));
>>> >> >> >> >
>>> >> >> >> >When setCallback is implemented like this:
>>> >> >> >> >
>>> >> >> >> >public AsyncCallbackDispatcher<T, R> setCallback(Object
>>>useless)
>>> >>{
>>> >> >> >> >
>>> >> >> >> > return this;
>>> >> >> >> >
>>> >> >> >> > }
>>> >> >> >> >
>>> >> >> >> >Why not just this?:
>>> >> >> >> >
>>> >> >> >> >caller.getTarget().createVolumeFromBaseImageCallBack(null,
>>> null);
>>> >> >> >> >
>>> >> >> >> >Thanks!
>>> >> >> >> >
>>> >> >> >> >--
>>> >> >> >> >*Mike Tutkowski*
>>> >> >> >> >*Senior CloudStack Developer, SolidFire Inc.*
>>> >> >> >> >e: mike.tutkow...@solidfire.com
>>> >> >> >> >o: 303.746.7302
>>> >> >> >> >Advancing the way the world uses the
>>> >> >> >> >cloud<http://solidfire.com/solution/overview/?video=play>
>>> >> >> >> >* *
>>> >> >> >>
>>> >> >> >>
>>> >> >> >
>>> >> >> >
>>> >> >> >--
>>> >> >> >*Mike Tutkowski*
>>> >> >> >*Senior CloudStack Developer, SolidFire Inc.*
>>> >> >> >e: mike.tutkow...@solidfire.com
>>> >> >> >o: 303.746.7302
>>> >> >> >Advancing the way the world uses the
>>> >> >> >cloud<http://solidfire.com/solution/overview/?video=play>
>>> >> >> >*(tm)*
>>> >> >>
>>> >> >>
>>> >> >
>>> >> >
>>> >> >--
>>> >> >*Mike Tutkowski*
>>> >> >*Senior CloudStack Developer, SolidFire Inc.*
>>> >> >e: mike.tutkow...@solidfire.com
>>> >> >o: 303.746.7302
>>> >> >Advancing the way the world uses the
>>> >> >cloud<http://solidfire.com/solution/overview/?video=play>
>>> >> >*(tm)*
>>> >>
>>> >>
>>> >
>>> >
>>> >--
>>> >*Mike Tutkowski*
>>> >*Senior CloudStack Developer, SolidFire Inc.*
>>> >e: mike.tutkow...@solidfire.com
>>> >o: 303.746.7302
>>> >Advancing the way the world uses the
>>> >cloud<http://solidfire.com/solution/overview/?video=play>
>>> >*(tm)*
>>>
>>>
>>
>>
>> --
>> *Mike Tutkowski*
>> *Senior CloudStack Developer, SolidFire Inc.*
>> e: mike.tutkow...@solidfire.com
>> o: 303.746.7302
>> Advancing the way the world uses the
>>cloud<http://solidfire.com/solution/overview/?video=play>
>> *(tm)*
>>
>
>
>
>--
>*Mike Tutkowski*
>*Senior CloudStack Developer, SolidFire Inc.*
>e: mike.tutkow...@solidfire.com
>o: 303.746.7302
>Advancing the way the world uses the
>cloud<http://solidfire.com/solution/overview/?video=play>
>*(tm)*
