----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29437/#review66990 -----------------------------------------------------------
Hi Nishant, thanks for the updated review. Just FYI, our preferred patches are actually small and logical commits. So in your example, is one patch just for executor registration timeout, and another review for executor launch timeout. And you can depend one review on top of another. The larger the patch the harder it is to accept and commit it. - Timothy Chen On Jan. 7, 2015, 6:41 a.m., Nishant Suneja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29437/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2015, 6:41 a.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-999 > https://issues.apache.org/jira/browse/MESOS-999 > > > Repository: mesos-git > > > Description > ------- > > As part of this bug fix, I have trigerred the executor registration timeout > timer after the container's future object is set, instead of starting the > timer when the container launch is still pending. > > Also, a new executor launch timer has been added. This timer gates the time > in which a successful executor container launch should happen. The executor > registration timer starts after the successful container launch. > > > Diffs > ----- > > src/slave/constants.hpp fd1c1ab > src/slave/constants.cpp 2a99b11 > src/slave/flags.hpp 670997d > src/slave/slave.hpp 70bd8c1 > src/slave/slave.cpp 50b5781 > src/tests/composing_containerizer_tests.cpp 5ab5a36 > src/tests/containerizer.hpp 24b014f > src/tests/slave_tests.cpp f2896a1 > > Diff: https://reviews.apache.org/r/29437/diff/ > > > Testing > ------- > > Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger > make check succeeds. > > > Thanks, > > Nishant Suneja > >
