> On Jan. 13, 2015, 11:29 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 1380-1381
> > <https://reviews.apache.org/r/29869/diff/1/?file=820049#file820049line1380>
> >
> > Curious, why you want acceptOffers() call into this method instead of
> > the current way. I imagine each operation to be implemented as a
> > function/method, similar to what you did in Master::call().
>
> Ben Mahler wrote:
> Yep, that's what we're aiming for, we're thinking of running through the
> operations and creating a chain of computation.
>
> Vinod Kone wrote:
> maybe i'm reading the comment wrong, but isn't the end goal here not to
> remove Master::launchTasks() method but to remove the installation of
> LaunchTasksMessage? in my mind, the fully implemented receive() will look
> like this:
>
> ```
> void Master::receive(
> const UPID& from,
> const FrameworkInfo& frameworkInfo,
> const scheduler::Call::Accept& accept) {
>
> for (const Operation operation, accept.operations()) {
> switch (operation.type()) {
> case Offer::Operation::LAUNCH:
> launchTasks(...);
> break;
> case Offer::Operation::RESERVE:
> reserveResources(...);
> break;
> case Offer::Operation::UNRESERVE:
> unreserveResources(...);
> break;
> case Offer::Operation::Create:
> createVolume(...);
> break;
> case Offer::Operation::Destroy:
> destroyVolume(...);
> break;
> }
> }
> }
> ```
Not sure if we can structure it that way, given `launchTasks` is asynchronous
and we likely want to respect the ordering of operations.
We might want to chain the operations, for example:
```
Future<Nothing> chain;
for (const Operation operation, accept.operations()) {
switch (operation.type()) {
case Offer::Operation::LAUNCH:
chain.then(defer(self(), &Master::launchTasks, ...)); // Returns a Future
once the task is launched fully.
break;
case Offer::Operation::RESERVE:
chain.then(defer(self(), &Master::reserveResources, ...));
break;
case Offer::Operation::UNRESERVE:
chain.then(defer(self(), &Master::unreserveResources, ...));
break;
case Offer::Operation::Create:
chain.then(defer(self(), &Master::createVolume, ...));
break;
case Offer::Operation::Destroy:
chain.then(defer(self(), &Master::destroyVolume, ...));
break;
}
}
```
I'm not sure and it needs some exploration. We might be able to go with
something like you suggested if there's enough happening synchronously.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29869/#review67970
-----------------------------------------------------------
On Jan. 14, 2015, 1:25 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29869/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2015, 1:25 a.m.)
>
>
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is an initial stub that only handles accepting offers.
>
> The plan (per the TODO) is to have a single path for launching tasks through
> `acceptOffers`, as opposed to `launchTasks`.
>
>
> Diffs
> -----
>
> src/master/master.hpp 26116aff1e965501c8d94ea0b5bd1be37f944887
> src/master/master.cpp 63ca19ab9618feccd93a2335f9287122a4665c5e
>
> Diff: https://reviews.apache.org/r/29869/diff/
>
>
> Testing
> -------
>
> The code path currently cannot be executed. Tests will be added once the
> scheduler driver supports accepting offers.
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
