> On March 17, 2016, 2:27 a.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; > ``` > > Greg Mann wrote: > 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?
Agree; updated the patches for that :). - Klaus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/#review123903 ----------------------------------------------------------- On March 18, 2016, 2:47 p.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37168/ > ----------------------------------------------------------- > > (Updated March 18, 2016, 2:47 p.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 > >
