----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48387/#review138478 -----------------------------------------------------------
Fix it, then Ship it! Thanks for your patience. This looks good! Some other suggestions: - Regarding the changes in tests, we can just resume the clock. This is much more simpler/intuitive. - The newly introduced delay makes the tests run a lot slower sometimes ~1 sec! Let's default the delay to `0ms` for the tests. I have included the changes from the above suggestions in this diff: https://gist.github.com/hatred/171deb74c3b28b13fc5e681fb697a64c I would be happy to commit the changes once you post the updated diff from the gist + the other minor issues I raised below. src/scheduler/constants.hpp (line 30) <https://reviews.apache.org/r/48387/#comment203653> Nit: Extra new line src/scheduler/flags.hpp (line 49) <https://reviews.apache.org/r/48387/#comment203652> Nit: Extra new line src/scheduler/scheduler.cpp (line 141) <https://reviews.apache.org/r/48387/#comment203654> Pass by const ref. `const Flags&` src/scheduler/scheduler.cpp (line 307) <https://reviews.apache.org/r/48387/#comment203655> It is possible to compare a `Option<T>` directly with `T`. You can just do `connectionId != _connectionId`. src/scheduler/scheduler.cpp (line 461) <https://reviews.apache.org/r/48387/#comment203659> Nit: Kill this extra new line. src/scheduler/scheduler.cpp (line 470) <https://reviews.apache.org/r/48387/#comment203656> Nit: new line before. We typically have a newline after a statement spanning multiple lines. src/scheduler/scheduler.cpp (lines 747 - 751) <https://reviews.apache.org/r/48387/#comment203657> Kill this comment. It is no longer relevant. We are no longer loading from `local::Flags`. src/scheduler/scheduler.cpp (line 765) <https://reviews.apache.org/r/48387/#comment203658> Nit: Kill this new line. - Anand Mazumdar On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48387/ > ----------------------------------------------------------- > > (Updated June 19, 2016, 7:08 p.m.) > > > Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone. > > > Bugs: MESOS-5359 > https://issues.apache.org/jira/browse/MESOS-5359 > > > Repository: mesos > > > Description > ------- > > The scheduler library has been updated to wait a little deley before > initiate a connection with the master. The maximum amount of time waited > by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After > the master has been detected the scheduler picks a random delay that > can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359 > > > Diffs > ----- > > src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 > src/scheduler/constants.hpp PRE-CREATION > src/scheduler/flags.hpp PRE-CREATION > src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 > src/tests/http_fault_tolerance_tests.cpp > baa07395b9bd588daa5438369954584787d7952a > src/tests/master_maintenance_tests.cpp > a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c > > Diff: https://reviews.apache.org/r/48387/diff/ > > > Testing > ------- > > mesos-tests > > > Thanks, > > Jose Guilherme Vanz > >
