----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22066/#review51039 -----------------------------------------------------------
Ok, almost there! One main suggestion: can this flag be optional and disabled by default for now? This might cause trouble for large clusters per my comment below. If disabled by default initially, we can safely test turning on this flag for large clusters, and we don't need to send a big heads up to the user/dev mailing lists about this new behavior. src/master/flags.hpp <https://reviews.apache.org/r/22066/#comment88916> Can we have this as an optional flag that is disabled initially? My concern is that this will cause a lot of additional offer churn for large clusters (10,000s of slaves). Especially with 5 minutes! Let's add a TODO here that this description will need to be updated when we have optimistic offers: E.g.: // TODO(karya): When we have optimistic offers, this will only // benefit frameworks that accidentally lose an offer. src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88927> "an offer" :) src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88928> This comment is no longer relevant, and we can remove the Clock::resume src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88932> /s/ / / src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88929> We don't need to resume it, settling will ensure that any pending timers / events are processed. src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88931> "be rescinded" src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88933> No need to resume per comment above. src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88930> "be rescinded" src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88935> It's not the slave that declines, it's the framework. How about: "Wait for the framework to decline the offers." src/tests/master_tests.cpp <https://reviews.apache.org/r/22066/#comment88934> No need to resume per comment above. - Ben Mahler On Aug. 18, 2014, 9:37 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22066/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2014, 9:37 p.m.) > > > Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Timothy > Chen. > > > Bugs: mesos-186 > https://issues.apache.org/jira/browse/mesos-186 > > > Repository: mesos-git > > > Description > ------- > > A timer is associated with each newly created offer. > The offer is rescinded on timeout. > The timer is disarmed on a launchTask or decline. > > This is continuation of Tim's patch: https://reviews.apache.org/r/22796/. > Revision two represents the latest patch from Tim. I have tried to address > the comments/concerns from that request into the third revision. > > > Diffs > ----- > > src/master/flags.hpp 5e9ecb5 > src/master/master.hpp c9f989a > src/master/master.cpp e948803 > src/tests/master_tests.cpp 9de2424 > > Diff: https://reviews.apache.org/r/22066/diff/ > > > Testing > ------- > > Added one more test after Tim's patch to test offer is not rescinded once it > has been declined. > > make check > > > Thanks, > > Kapil Arya > >
