----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14669/#review32750 -----------------------------------------------------------
Ship it! Looks great! Thanks for the patience. Let me know what you think about my comment in sched.cpp. src/master/master.hpp <https://reviews.apache.org/r/14669/#comment61711> s/_offerIds/offerIds/ src/master/master.hpp <https://reviews.apache.org/r/14669/#comment61712> lets just do return executors.contains(frameworkId) && executors.get(frameworkId).contains(executorId); src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61728> This needs to be rephrased now that we take multiple offers. maybe s/offer/'launchTasks()'/ ? src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61727> s/was/is/ ? src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61730> s/OfferChecker/ValidOfferChecker/ ? src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61731> why not stringify the offers as you did below? src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61737> why const? src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61738> new line. src/master/master.cpp <https://reviews.apache.org/r/14669/#comment61739> please include offerError.get() here to make it easy for frameworks to understand what is invalid. src/sched/sched.cpp <https://reviews.apache.org/r/14669/#comment61724> This change means, if the scheduler gets deployed first it won't work an old master. This is a bit concerning especially because it is not going to work even if it is using the old api. One solution is to have both old and new versions of launchTasks() in the scheduler process. Thoughts? - Vinod Kone On Jan. 17, 2014, 12:05 a.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14669/ > ----------------------------------------------------------- > > (Updated Jan. 17, 2014, 12:05 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-749 > https://issues.apache.org/jira/browse/MESOS-749 > > > Repository: mesos-git > > > Description > ------- > > Running tasks on more than one offer belonging to a single slave can be > useful in situations with multiple out-standing offers. > > This patch extends the usual launchTasks() to accept a vector of OfferIDs. > The previous launchTasks (accepting a single OfferID) has been kept for > backward compatibility, but this now calls the new launchTasks() with a > one-element list. > This also applied for the JNI and python interfaces, which accepts both > formats as well. > > Offers are verified to belong to the same slave and framework, before > resources are merged and used. > > > Diffs > ----- > > include/mesos/scheduler.hpp 8063997 > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 9869929 > src/java/src/org/apache/mesos/MesosSchedulerDriver.java ed4b4a3 > src/java/src/org/apache/mesos/SchedulerDriver.java 5b0ca39 > src/master/master.hpp 18a6cc4 > src/master/master.cpp 008033e > src/messages/messages.proto 1f264d5 > src/python/native/mesos_scheduler_driver_impl.cpp 059ed5d > src/sched/sched.cpp f9028e8 > src/tests/master_tests.cpp d34450b > src/tests/resource_offers_tests.cpp 9beb949 > > Diff: https://reviews.apache.org/r/14669/diff/ > > > Testing > ------- > > Three new tests has been added: LaunchCombinedOfferTest, > LaunchAcrossSlavesTest and LaunchDuplicateOfferTest > This test ensures that: > 1) Multiple offers can be used to run a single task (requesting the sum of > offer resources). > 2) Offers cannot span multiple slaves. > 3) No offers can appear more than once in offer list. > > $ make check > ... > [ RUN ] MasterTest.LaunchCombinedOfferTest > [ OK ] MasterTest.LaunchCombinedOfferTest (2010 ms) > [ RUN ] MasterTest.LaunchAcrossSlavesTest > [ OK ] MasterTest.LaunchAcrossSlavesTest (3 ms) > [ RUN ] MasterTest.LaunchDuplicateOfferTest > [ OK ] MasterTest.LaunchDuplicateOfferTest (3 ms) > ... > > > Thanks, > > Niklas Nielsen > >
