mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 37168: Add an example framework using dynamic reservation.
Date Thu, 17 Mar 2016 15:54:25 GMT


> On March 16, 2016, 6:27 p.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 119
> > <https://reviews.apache.org/r/37168/diff/12/?file=1300732#file1300732line119>
> >
> >     Could you clarify for me how the slave gets into the RESERVED state the first
time? It seems to me we should have something here similar to what's in the switch for `State::UNRESERVING`,
i.e., check the offer and see if the reserved resources are contained in it? The framework
test passes for me though, so it seems to be working nonetheless?
> 
> Klaus Ma wrote:
>     Good catch! I think I missed `updateState(dispatched, offer.slave_id(), State::RESERVED);`
just before launching task; it works because both `State::RESERVED` and `State::RESERVING`
are using the same logic.
>     
>     When slave transfer from `State::RESERVING` to `State::RESERVED`, it's better to
launch tasks directly; otherwise, we have to wait for another allocation interval to launch
task. I used to try following code to seperate action in those two state, but `case State::RESERVING:`
without `break;` is not a good style :).
>     
>     ```
>             case State::RESERVING:
>               {
>                 Resources resources = offer.resources();
>                 Resources reserved = resources.reserved(role);
>     
>                 if (reserved.contains(taskResources)) {
>                   updateState(dispatched, offer.slave_id(), State::RESERVED);
>                 }
>               }
>               // NOTE: No `break;`. Launch task after transfering to
>               // `State::RESERVED`.
>             case State::RESERVED:
>               {
>                 if (tasksLaunched == totalTasks) {
>                   // If all tasks were launched, unreserve those resources.
>                   Try<Nothing> reserved = unreserveResources(driver, offer);
>                   updateState(reserved, offer.slave_id(), State::UNRESERVING);
>                 } else if (tasksLaunched < totalTasks) {
>                   // Framework dispatches task on the reserved resources.
>                   Try<Nothing> dispatched = dispatchTasks(driver, offer);
>                   updateState(dispatched, offer.slave_id(), State::TASK_RUNNING);
>                 }
>               }
>               break;
>     ```

Ah yea, this makes sense :-) Thanks for the explanation! Do you think you could add a comment
to the code here to explain this reasoning?


- Greg


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


On March 17, 2016, 6:15 a.m., Klaus Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> -----------------------------------------------------------
> 
> (Updated March 17, 2016, 6:15 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
>     https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>


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