Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29437/#review141469 --- Closing this review due to inactivity. Please see our [guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md) for reopening reviews. - Joris Van Remoortere On Jan. 8, 2015, 7:54 a.m., Nishant Suneja wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29437/ > --- > > (Updated Jan. 8, 2015, 7:54 a.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-999 > https://issues.apache.org/jira/browse/MESOS-999 > > > Repository: mesos > > > 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 fd1c1aba0aa62372ab399bee5709ce81b8e92cec > src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd > src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef > src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 > src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b > src/tests/composing_containerizer_tests.cpp > 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad > src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 > src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 > > Diff: https://reviews.apache.org/r/29437/diff/ > > > Testing > --- > > Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger > make check succeeds. > > > Thanks, > > Nishant Suneja > >
Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully
> On July 14, 2015, 10:21 a.m., Timothy Chen wrote: > > Nishant are you still around to help rebase this? Sorry I think we dropped > > this review somehow. He responded on https://issues.apache.org/jira/browse/MESOS-999. I did some work on it but ended up not pushing it. Would you like to comment on the ticket? - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29437/#review91637 --- On Jan. 7, 2015, 11:54 p.m., Nishant Suneja wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29437/ > --- > > (Updated Jan. 7, 2015, 11:54 p.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-999 > https://issues.apache.org/jira/browse/MESOS-999 > > > Repository: mesos > > > 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 fd1c1aba0aa62372ab399bee5709ce81b8e92cec > src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd > src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef > src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 > src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b > src/tests/composing_containerizer_tests.cpp > 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad > src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 > src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 > > Diff: https://reviews.apache.org/r/29437/diff/ > > > Testing > --- > > Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger > make check succeeds. > > > Thanks, > > Nishant Suneja > >
Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29437/#review91637 --- Nishant are you still around to help rebase this? Sorry I think we dropped this review somehow. - Timothy Chen On Jan. 8, 2015, 7:54 a.m., Nishant Suneja wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29437/ > --- > > (Updated Jan. 8, 2015, 7:54 a.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-999 > https://issues.apache.org/jira/browse/MESOS-999 > > > Repository: mesos > > > 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 fd1c1aba0aa62372ab399bee5709ce81b8e92cec > src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd > src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef > src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 > src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b > src/tests/composing_containerizer_tests.cpp > 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad > src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 > src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 > > Diff: https://reviews.apache.org/r/29437/diff/ > > > Testing > --- > > Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger > make check succeeds. > > > Thanks, > > Nishant Suneja > >
Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29437/#review87245 --- @tnachen: What is blocked this? Looks like it got a ship it, but stopped review-cycles somehow? - Niklas Nielsen On Jan. 7, 2015, 11:54 p.m., Nishant Suneja wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29437/ > --- > > (Updated Jan. 7, 2015, 11:54 p.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-999 > https://issues.apache.org/jira/browse/MESOS-999 > > > Repository: mesos > > > 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 fd1c1aba0aa62372ab399bee5709ce81b8e92cec > src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd > src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef > src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 > src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b > src/tests/composing_containerizer_tests.cpp > 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad > src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 > src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 > > Diff: https://reviews.apache.org/r/29437/diff/ > > > Testing > --- > > Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger > Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger > make check succeeds. > > > Thanks, > > Nishant Suneja > >