----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45067/#review126646 -----------------------------------------------------------
src/examples/long_lived_framework.cpp (line 51) <https://reviews.apache.org/r/45067/#comment189655> I think you should add a comment here about what the high level behavior of this scheduler is. For example that this schedule tries to pick one slave and launch as many tasks on it as possible, using a single multi-task executor. And that if the slave or executor fails, it picks another slave and repeats the process. src/examples/long_lived_framework.cpp (line 64) <https://reviews.apache.org/r/45067/#comment189650> Any particular reason you are using "cout" instead of LOG? It would be great to have timestamps, esp if you are planning to run this in a test cluster. src/examples/long_lived_framework.cpp (line 86) <https://reviews.apache.org/r/45067/#comment189653> Can we use "foreach(const Offer& offer, offers)" here? src/examples/long_lived_framework.cpp (line 87) <https://reviews.apache.org/r/45067/#comment189652> why do you need a vector here? AFAICT, you only ever call launchTasks with a single task. If that's the case just pass '{task}' to driver.launchTasks(). src/examples/long_lived_framework.cpp (line 92) <https://reviews.apache.org/r/45067/#comment189651> s/offer/offer is/ src/examples/long_lived_framework.cpp (lines 104 - 150) <https://reviews.apache.org/r/45067/#comment189656> This is still a bit hard to follow. How about: ``` foreach (const Offer& offer : offers) { if (slaveID.isNone() { // No active executor running in the cluster. Launch a new task with executor. if (offer.resources < (task + executor).resources()) { // Not enough resources. Decline. decline(); } else { launch(); } } else if (slaveID == offer.slaveID() { // Offer from the same slave that has an active executor. Launch more tasks on that executor. if (offer.resources < task.executors()) { // Noe enough resources. Decline. decline(); } else { launch(); } } else { // Offer from a slave different from the slave that has an active executor. Decline. decline(); } } ``` src/examples/long_lived_framework.cpp (line 177) <https://reviews.apache.org/r/45067/#comment189649> log a message here. src/examples/long_lived_framework.cpp (line 186) <https://reviews.apache.org/r/45067/#comment189648> log a message here. - Vinod Kone On March 29, 2016, 7:08 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45067/ > ----------------------------------------------------------- > > (Updated March 29, 2016, 7:08 p.m.) > > > Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and > Vinod Kone. > > > Bugs: MESOS-5062 > https://issues.apache.org/jira/browse/MESOS-5062 > > > Repository: mesos > > > Description > ------- > > This gives the example `long-lived-framework` enough options to run outside > of the build environment. > > This also updates: > > * The style of the framework code. > * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators). > * Restricts the framework to one agent. Otherwise, it would grab a small > chunk of every machine in the cluster. > * Adds filters for declined offers. > > > Diffs > ----- > > src/examples/long_lived_framework.cpp > ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 > > Diff: https://reviews.apache.org/r/45067/diff/ > > > Testing > ------- > > make check > > Ran this on the master node on a Mesos cluster: > ``` > ./long-lived-framework --master=zk://localhost:2181/mesos > --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; > --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && > ./long-lived-executor" > ``` > > > Thanks, > > Joseph Wu > >
