mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 29976: Moved launchTasks logics to ACCEPT call handler in master.
Date Thu, 22 Jan 2015 18:14:57 GMT


> On Jan. 21, 2015, 11:31 p.m., Ben Mahler wrote:
> > src/master/master.hpp, line 429
> > <https://reviews.apache.org/r/29976/diff/3/?file=828919#file828919line429>
> >
> >     Since this is in other places in the master code, might be nice to follow up
on a sweep of the following:
> >     
> >     s/offeredResources/offered/
> >     s/usedResources/used/

Sure, will follow up with a patch.


> On Jan. 21, 2015, 11:31 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2766
> > <https://reviews.apache.org/r/29976/diff/3/?file=828920#file828920line2766>
> >
> >     Not yours, mind doing s/offerValidators/validators/ ?
> >     
> >     Seems consistent with the singular 'validator' below as well.
> >     
> >     Feel free to do this as a follow up.

Done.


> On Jan. 21, 2015, 11:31 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 2804-2805
> > <https://reviews.apache.org/r/29976/diff/3/?file=828920#file828920line2804>
> >
> >     Could you leverage `drop` here per my other comment? That way, we'll log consistently
when we drop things, regardless of when we drop them. We'll also get consistent metrics about
dropped messages! Thoughts?
> >     
> >     You can add a `drop` overload for Accept:
> >     ```
> >     drop(accept, frameworkInfo, "Framework cannot be found");
> >     ```
> >     
> >     Consider a TODO on this version of `drop` for dealing with recovering resources
from the offers, notice how this is becoming similar to scheduler.cpp! :)
> >     
> >     Would really like to have the framework info here for consistent logging.

Added a TODO. THanks for the comment. I like it.


> On Jan. 21, 2015, 11:31 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 2894-2896
> > <https://reviews.apache.org/r/29976/diff/3/?file=828920#file828920line2894>
> >
> >     Ditto for `drop` here.
> >     
> >     ```
> >     drop(accept, frameworkInfo, "Framework cannot be found");
> >     ```
> >     
> >     It's not clear if recovering the resources just below here will ever be necessary,
since we don't know the framework! It seems nice for `drop(Accept, ...)` to consistently handle
resource recovery, if applicable.
> >     
> >     We could even consider `drop(Accept, ...)` also dealing with lost task notifications,
through additional overloads (possibly per operation to cleanup the cruft in the Offer::Operation::LAUNCH
case block below), but one step at a time, a TODO for this latter part would be great :)

Left a TODO.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29976/#review69066
-----------------------------------------------------------


On Jan. 21, 2015, 10:29 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29976/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 10:29 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Moved launchTasks logics to ACCEPT call handler in master.
> 
> This patch moves all logics of launchTasks in master to the ACCEPT call handler in master,
and leaves stubs for other offer operations. We still keeps the launchTasks handler for backwards
compatibility. launchTasks will simply create an Accept call message, and invoke the ACCEPT
call handler.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp a8ce4d023ddea36cb83a2dc7b95abd12342f345a 
>   src/master/master.cpp e9dcca3c92c94f3123519855e238bcef47eeece9 
>   src/tests/resource_offers_tests.cpp d098e7016ac0da7f1d629af099bb1b8fa66da839 
> 
> Diff: https://reviews.apache.org/r/29976/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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