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 06:56:42 GMT
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)*

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