You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2016/12/12 16:02:46 UTC
Review Request 54664: Acked correct task id in
DefaultExecutorTest.KillTaskGroupOnTaskFailure.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54664/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar and Vinod Kone.
Bugs: MESOS-6744
https://issues.apache.org/jira/browse/MESOS-6744
Repository: mesos
Description
-------
Acked correct task id in DefaultExecutorTest.KillTaskGroupOnTaskFailure.
Diffs
-----
src/tests/default_executor_tests.cpp a88796b83c17fb01e7698907e9b0899a63700782
Diff: https://reviews.apache.org/r/54664/diff/
Testing
-------
Test did not fail in my setup in 4000 iterations.
Thanks,
Benjamin Bannier
Re: Review Request 54664: Acked correct task id in
DefaultExecutorTest.KillTaskGroupOnTaskFailure.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54664/#review158926
-----------------------------------------------------------
Fix it, then Ship it!
Nice Catch!
- Can you update the earlier review in the chain based on the review comment later?
- Can you update the description with more details on how _your change_ fixes the test?
- Do we need a similar change for any of the tests you fixed in r54422?
src/tests/default_executor_tests.cpp (line 571)
<https://reviews.apache.org/r/54664/#comment229767>
Nit: Leave a newline after this multi line statement.
Ditto for the other occurence.
- Anand Mazumdar
On Dec. 12, 2016, 4:02 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54664/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 4:02 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6744
> https://issues.apache.org/jira/browse/MESOS-6744
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Acked correct task id in DefaultExecutorTest.KillTaskGroupOnTaskFailure.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp a88796b83c17fb01e7698907e9b0899a63700782
>
> Diff: https://reviews.apache.org/r/54664/diff/
>
>
> Testing
> -------
>
> Test did not fail in my setup in 4000 iterations.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 54664: Acked correct task id in
DefaultExecutorTest.KillTaskGroupOnTaskFailure.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54664/
-----------------------------------------------------------
(Updated Dec. 13, 2016, 12:50 p.m.)
Review request for mesos, Anand Mazumdar and Vinod Kone.
Changes
-------
Sprinkle some vertical space goodness.
Bugs: MESOS-6744
https://issues.apache.org/jira/browse/MESOS-6744
Repository: mesos
Description (updated)
-------
When acknowledging task status updates, this test was erroneously
using a cached task id instead of the actual task id send in the
status update. This can lead to failed acknowledgements when
status updates appear in different order.
This patch just uses the task id sent as part of the status update in
the acknowledgement.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp a88796b83c17fb01e7698907e9b0899a63700782
Diff: https://reviews.apache.org/r/54664/diff/
Testing
-------
Test did not fail in my setup in 4000 iterations.
Thanks,
Benjamin Bannier
Re: Review Request 54664: Acked correct task id in
DefaultExecutorTest.KillTaskGroupOnTaskFailure.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54664/#review158961
-----------------------------------------------------------
Patch looks great!
Reviews applied: [54663, 54664]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 12, 2016, 4:02 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54664/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 4:02 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6744
> https://issues.apache.org/jira/browse/MESOS-6744
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Acked correct task id in DefaultExecutorTest.KillTaskGroupOnTaskFailure.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp a88796b83c17fb01e7698907e9b0899a63700782
>
> Diff: https://reviews.apache.org/r/54664/diff/
>
>
> Testing
> -------
>
> Test did not fail in my setup in 4000 iterations.
>
>
> Thanks,
>
> Benjamin Bannier
>
>