> On June 25, 2019, 7:13 p.m., Benjamin Mahler wrote: > > Thanks for the update. Why not update suppressOffers in this patch? What's > > the revert concern about? It seems we either need both updated in this > > patch or we need another patch for suppressOffers that immediately follows > > this. I would prefer to have this patch introduce the suppress roles set, > > which means updating reviveOffers/suppressOffers/updateFramework in this > > patch.
My initial concern was that some framework that depends on current behavior of `suppressOffers()` might turn up, and we will have to do something with this... Given that in a week's time noone raised such concerns, that the change needed to make `suppressOffers()` consistent with everything else is trivial, and that the implementation of per-role `suppressOffers()` heavily depends on this change anyway, I decided to update suppressOffers() here. > On June 25, 2019, 7:13 p.m., Benjamin Mahler wrote: > > include/mesos/scheduler.hpp > > Lines 295-297 (patched) > > <https://reviews.apache.org/r/70894/diff/2/?file=2152130#file2152130line295> > > > > Hm.. I don't understand what this TODO is about Dropped this TODO - the removal of offer filters has never been reliable in the V0 scheduler (like most other requests in V0), and this patch doesn't affect this. > On June 25, 2019, 7:13 p.m., Benjamin Mahler wrote: > > include/mesos/scheduler.hpp > > Lines 303-307 (patched) > > <https://reviews.apache.org/r/70894/diff/2/?file=2152130#file2152130line303> > > > > This is a little vague, do you mean to have it clear the suppressed > > roles list? It seems you've already expressed in the review commentary that > > you are planning to update this in a separate patch? > > > > Why not make them consistent in this patch? At the very least I would > > want to commit them together, it seems inconsistent to only update > > reviveOffers to clear the supppressed set, but not suppressOffers? Changed `suppressedOffers()` too - see above. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70894/#review216124 ----------------------------------------------------------- On June 26, 2019, 3:28 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70894/ > ----------------------------------------------------------- > > (Updated June 26, 2019, 3:28 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-9793 > https://issues.apache.org/jira/browse/MESOS-9793 > > > Repository: mesos > > > Description > ------- > > This patch adds a list of suppressed roles to the arguments of > scheduler driver's updateFramework() method to make it possible > for V0 frameworks to selectively suppress/revive offers. > > To keep re-registration conststent with updateFramework(), the required > set of suppressed roles is now stored by the driver and used when it > re-registers. > > To keep reviveOffers() consistent with re-registration, reviveOffers() > now clears the stored set and, when issued in a disconnected state of > the driver, forces it to perform UPDATE_FRAMEWORK call upon > re-connection. > > NOTE: suppressOffers() has never been consistent with re-registration > in this regard. Making it constent with re-registration might affect > the behavior of existing frameworks, and will be performed in a separate > patch. > > > Diffs > ----- > > include/mesos/scheduler.hpp 8c6774885ceb3b0644c32842a17fba39e2c3e472 > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp > a21aca23cbef27b3c54bf1ae5834cbb457608130 > src/java/src/org/apache/mesos/SchedulerDriver.java > ee5a9e24956299d84ff03a0fc5a22e641470a03f > src/python/interface/src/mesos/interface/__init__.py > f9642a0452e3161f3fd1b6a5d7ce0658d08c0b87 > src/sched/sched.cpp e6cc534264e3ff4edaa703a660afb2f896388802 > src/tests/master/update_framework_tests.cpp > 0d466e286adfd829bbe72cff9202860f7fcb043f > > > Diff: https://reviews.apache.org/r/70894/diff/3/ > > > Testing > ------- > > make check > `./bin/mesos-tests.sh --gtest_filter="UpdateFrameworkV0*" > --gtest_break_on_failure --gtest_repeat=1000` > > +a new test from the depending patch > > > Thanks, > > Andrei Sekretenko > >
