You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/07/14 19:21:54 UTC

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

Posted by Jiang Yan Xu <ya...@jxu.me>.

> 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
> 
>