> On June 25, 2019, 7:13 p.m., Benjamin Mahler wrote: > > include/mesos/scheduler.hpp > > Lines 290-293 (patched) > > <https://reviews.apache.org/r/70894/diff/2/?file=2152130#file2152130line290> > > > > // NOTE: if the framework is not connected to the master, the suppressed > > // roles set stored by the driver will be cleared and the next > > re-registration > > // will send an empty suppressed roles list. > > Andrei Sekretenko wrote: > Thanks! Fully agree that I need a better wording here. > > I'm afraid these two phrases might mislead someone someday: > - "next re-registration": it might be not clear to the reader if the > ongoing re-registrtation (SUBSCRIBE sent, but no FrameworkReregistered > received) is the "next" one or not. > - "will send an empty list": if, for example, > updateFramework()/suppress() is called immediately after reviveOffers(), the > set which the driver will set will be what the caller wants - but it will not > be empty. > > So I've tried to reword the comment (and remove implementation details) > without using these phrases.
typo: s/the set which the driver will **set**/the set which the driver will **send**/ - 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 > >
