Skip to content

Process dispatch#65

Closed
conqerAtapple wants to merge 3 commits intoapache:masterfrom
conqerAtapple:process_dispatch
Closed

Process dispatch#65
conqerAtapple wants to merge 3 commits intoapache:masterfrom
conqerAtapple:process_dispatch

Conversation

@conqerAtapple
Copy link
Contributor

Generic Asynchronous Dispatcher

Motivation

Since mesos code is based on the actor model and dispatching an interface
asynchronously is a large part of the code base, generalizing the higher level concept of
asynchronously dispatching an interface would eliminate the need to manually
program the dispatch boilerplate.

An example usage:
For a simple interface like:

class Interface                                                                  
{                                                                                
  virtual Future<size_t> writeToFile(const char* data) = 0;                      
  virtual ~Interface();                                                          
};     

Today the developer has to do the following:

a. Write a wrapper class that implements the same interface to add the
dispatching boilerplate.
b. Spend precious time in reviews.
c. Risk introducing bugs.

None of the above steps add any value to the executable binary.

The wrapper class would look like:

// -- hpp file                                                                   
class InterfaceProcess;                                                          

class InterfaceImpl : public Interface                                           
{                                                                                
public:                                                                          
  Try<Owned<InterfaceImpl>> create(const Flags& flags);                          

  virtual Future<size_t> writeToFile(const char* data);                          

  ~InterfaceImpl();
private:                                                                         
  Owned<InterfaceProcess> process;                                               
};                                                                               

// -- cpp file                                                                   
Try<Owned<InterfaceImpl>> create(const Flags& flags)                             
{                                                                                
  // Code to create the InterfaceProcess class.                                  
}                                                                                

Future Future<size_t> InterfaceImpl::writeToFile(const char* data)               
{                                                                                
  process->dispatch(                                                             
    &InterfaceProcess::writeToFile,                                              
    data);                                                                       
}                                                                                

InterfaceImpl::InterfaceImpl()                                                   
{                                                                                
  // Code to spawn the process                                                   
}                                                                                

InterfaceImpl::~InterfaceImpl()                                                  
{                                                                                
  // Code to stop the process.                                                   
}   

At the caller/client site, the code would look like:

Try<Owned<Interface>> in = InterfaceImpl::create(flags);                         
Future<size_t> result =                                                          
  in->writeToFile(data);                                                                       

Proposal

We should use C++'s rich language semnatics to express the intent and avoid
the boilerplate we write manually.
The basic intent of the code that leads to all the boilerplate above is:

a. An interface that provides a set of functionality.
b. An implementation of the interface.
c. Ability to dispatch that interface asynchronously using actor.

C++ has a rich set of generics that can be used to express above.

Components

  1. ProcessDispatcher
    This component will "dispatch" an interface implementation asychronously using
    the process framework.
    This component can be expressed as:

    ProcessDispatcher<Interface, InterfaceImplmentation>   
  2. DispatchInterface
    Any interface that provides an implementation that can be "dispatched" can be
    expressed using this component.
    This component can be expressed as:

Dispatchable<Interface>  

Usage:

  1. Simple usage
Try<Owned<Dispatchable<Interface>>> dispatcher =                                 
  ProcessDispatcher<Interface, InterfaceImpl>::create(flags);                    

Future<size_t> result =                                                          
  dispatcher->dispatch(                                                          
    Interface::writeToFile,                                                      
    data);                                                                       
  1. Collecting the interface in a container

    vector<Owned<Dispatchable<Interface>>> dispatchCollection;                       
    
    Try<Owned<Dispatchable<Interface>>> dispatcher1 =                                
    ProcessDispatcher<Interface, InterfaceImpl1>::create(flags);                   
    
    Try<Owned<Dispatchable<Interface>>> dispatcher2 =                                
    ProcessDispatcher<Interface, InterfaceImpl2>::create("test");                  
    
    dispatchCollection.push_back(dispatcher1);                                       
    dispatchCollection.push_back(dispatcher2);    

The advantages of using the generic dispatcher:

  • Saves time by avoiding to write all the boilerplate and going through review
    cycles.
  • Less bugs.
  • Focus on real problem and not boilerplate.
  • Less code to represent a primitive.

@adam-mesos
Copy link
Contributor

@conqer Could you please provide a description and a link to a JIRA so we can understand what you're proposing/fixing here?

@conqerAtapple
Copy link
Contributor Author

@adam-mesos, @mpark I have updated the description.

@haosdent
Copy link
Member

This idea looks cool. 👍

@conqerAtapple
Copy link
Contributor Author

@haosdent What do you think about the usage difference. Earlier at the call site it used to be:
interface->foo(arg1, arg2);

With this change, the interface would be:
dispatcher.dispatch(&interface::foo, arg1, arg2);

@conqerAtapple
Copy link
Contributor Author

@jieyu

@jieyu
Copy link
Member

jieyu commented Sep 17, 2015

@conqer I had a related idea years ago. Some discussion and code can be found here:
https://reviews.apache.org/r/14686

The goal is slightly different. You want to get rid of those 'dispatch' boilerplate code. My goal is to create a common base class for those libprocess Process wrapper classes so that we don't need to pass raw pointer (or smart pointer) around. It's safe to copy the wrapper class.

Looking at your examples, the following code hurts the readability a bit. We'll be having a lot of 'dispatch' calls in the code base, at which moment, people might be asking why not just use the raw dispatch (i.e., dispatch(process, xxx, args...);) instead?

Future<size_t> result =                                                          
  dispatcher->dispatch(                                                          
    Interface::writeToFile,                                                      
    data);

If would be cool if we can do the following directly. But I guess it not quite possible.

Future<size_t> result = dispatcher->writeToFile(data);

@conqerAtapple
Copy link
Contributor Author

@jieyu I agree that there is a inconvenience factor in writing "dispatch". The advantage is that you just write the process implementation class (no wrapper) and life cycle management comes for free. I am guessing that writing hundreds of lines of wrapper is more inconvenient.

@jieyu
Copy link
Member

jieyu commented Sep 17, 2015

@conqer It's a trade off, right? User has to use 'dispatch' at very single call site vs. only has one dispatch in the boilerplate code. I personally prefer the second one to simply the call sites.

@conqerAtapple
Copy link
Contributor Author

I see your point about the familiarity of using c++ syntax to call a
method. Since I have been writing both the caller sites and callee
code(wrapper), i feel that i might be happy trading off adapting my call
style to avoid writing 100s of boilerplate. If this was an external facing
API, I would not have second thoughts about using C++ language syntax. Here
the caller and callee is usually the same developer and developers usually
avoid boilerplate. WDYT?

On Thu, Sep 17, 2015 at 3:26 PM, Jie Yu notifications@github.com wrote:

@conqer https://github.com/conqer It's a trade off, right? User has to
use 'dispatch' at very single call site vs. only has one dispatch in the
boilerplate code. I personally prefer the second one to simply the call
sites.


Reply to this email directly or view it on GitHub
#65 (comment).

Jojy G Varghese

@jieyu
Copy link
Member

jieyu commented Sep 17, 2015

If you are OK with using dispatch(&TokenManager::xxx, args...) at the callsite, why not just use TokenManagerProcess directly? Why do you need TokenManager?

@conqerAtapple
Copy link
Contributor Author

Couple of reasons:

  • Conceptually I will be tightly coupled with the dispatch mechanism
    (process)
  • Life cycle management and process'ness of the implementation will also
    have to be done at the client site. By process'ness i mean process start,
    end, terminate etc. In a way this is the same point as the above point
    (tight coupling).

On Thu, Sep 17, 2015 at 3:56 PM, Jie Yu notifications@github.com wrote:

If you are OK with using dispatch(&TokenManager::xxx, args...) at the
callsite, why not just use TokenManagerProcess directly? Why do you need
TokenManager?


Reply to this email directly or view it on GitHub
#65 (comment).

Jojy G Varghese

@jieyu
Copy link
Member

jieyu commented Sep 17, 2015

Yeah, I think the lifecycle management part is definitely useful.

I am just not very convinced about the 'dispatch' part. I could be wrong. Maybe you should ping BenH.

@conqerAtapple
Copy link
Contributor Author

Yes i think it will come down to :

  • Inconvenience in adapting to the dispatch way of calling things

vs

  • getting rid of all the boilerplate
  • representation of dispatch primitive
  • Automatic life cycle management.

thanks for your feedback.

On Thu, Sep 17, 2015 at 4:29 PM, Jie Yu notifications@github.com wrote:

Yeah, I think the lifecycle management part is definitely useful.

I am just not very convinced about the 'dispatch' part. I could be wrong.
Maybe you should ping BenH.


Reply to this email directly or view it on GitHub
#65 (comment).

Jojy G Varghese

@jieyu
Copy link
Member

jieyu commented Sep 18, 2015

@conqer Thanks for the summary! I think we all agree that "Automatic life cycle management." is a useful thing to do (as you already did). We can iterate on the "dispatch" primitive later and see what other people think about this.

@conqerAtapple
Copy link
Contributor Author

Wanted to clarify what i meant by representation of dispatch as a higher level construct. Process "Dispatch" is A policy of how a interface can be dispatched. There could be others - say C++11 async or maybe grand central dispatch. The thought is along the lines of policy based designs.

On Sep 18, 2015, at 10:26 AM, Jie Yu notifications@github.com wrote:

@conqer Thanks for the summary! I think we all agree that "Automatic life cycle management." is a useful thing to do (as you already did). We can iterate on the "dispatch" primitive later and see what other people think about this.


Reply to this email directly or view it on GitHub.

@vinodkone
Copy link
Contributor

Closing stale PR.

@vinodkone vinodkone closed this Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants