You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/01/08 20:00:37 UTC
Re: Review Request 29718: 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/29718/#review67249
-----------------------------------------------------------
src/slave/slave.cpp
<https://reviews.apache.org/r/29718/#comment111221>
Let's take off MESOS-999, since all the details you want to describe is already in the comments.
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111220>
Space and captialize
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111219>
Space and captialize
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111214>
Not needed.
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111213>
Comment not needed.
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111215>
Not needed as well.
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111216>
Let's leave a space before this comment.
And captialize the first word.
// Wait.......
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111217>
Captialize the first word of the comment and all other comments please.
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/29718/#comment111218>
Space before comment.
- Timothy Chen
On Jan. 8, 2015, 6:43 p.m., Nishant Suneja wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29718/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 6:43 p.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.
>
>
> Diffs
> -----
>
> 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/29718/diff/
>
>
> Testing
> -------
>
> make check succeeds.
> Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger
>
>
> Thanks,
>
> Nishant Suneja
>
>
Re: Review Request 29718: Bug fix: Start the executor registration
timer, only when the container has launched successfully
Posted by Nishant Suneja <ni...@gmail.com>.
> On Jan. 8, 2015, 7 p.m., Timothy Chen wrote:
> > src/tests/slave_tests.cpp, line 1565
> > <https://reviews.apache.org/r/29718/diff/1/?file=813364#file813364line1565>
> >
> > Space and captialize
Which space are we talking about ?
Theres already a space between "//" and "ignore".
Is there some other place where you want a space ?
- Nishant
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29718/#review67249
-----------------------------------------------------------
On Jan. 9, 2015, 10:35 a.m., Nishant Suneja wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29718/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 10:35 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.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp b234f5359a91a293b93f97f035b08ef0a4c9b20d
> src/tests/composing_containerizer_tests.cpp 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad
> src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711
> src/tests/slave_tests.cpp a85a0282947278ce6e02e38093f8b88af4cb612a
>
> Diff: https://reviews.apache.org/r/29718/diff/
>
>
> Testing
> -------
>
> make check succeeds.
> Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger
>
>
> Thanks,
>
> Nishant Suneja
>
>