cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Tutkowski <mike.tutkow...@solidfire.com>
Subject Re: Callback pattern
Date Wed, 29 Jan 2014 07:16:16 GMT
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.tutkowski@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.yang@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.tutkowski@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.yang@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.tutkowski@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.yang@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.tutkowski@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.yang@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.tutkowski@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().createVolumeFromBaseImageCallB
>> >>>>>>>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.tutkowski@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.tutkowski@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.tutkowski@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.tutkowski@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.tutkowski@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.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*(tm)*

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message