You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2018/05/07 22:25:58 UTC

Review Request 66993: Removed unnecessary expectation on termination.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/
-----------------------------------------------------------

Review request for mesos, Andrei Budnik and James Peach.


Bugs: MESOS-8884
    https://issues.apache.org/jira/browse/MESOS-8884


Repository: mesos


Description
-------

This test was flaky because termination could already happened when we
set up the expectation. Given that we already verified task state, I do
not see checking container termination explicitly is necessary, so
removing the expectation should fix the flakiness.


Diffs
-----

  src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 


Diff: https://reviews.apache.org/r/66993/diff/1/


Testing
-------

GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose


Thanks,

Zhitao Li

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/#review202769
-----------------------------------------------------------



PASS: Mesos patch 66993 was successfully built and tested.

Reviews applied: `['66993']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66993

- Mesos Reviewbot Windows


On May 8, 2018, 8:36 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> -----------------------------------------------------------
> 
> (Updated May 8, 2018, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
>     https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/2/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/#review202750
-----------------------------------------------------------



Patch looks great!

Reviews applied: [66993]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 8, 2018, 8:36 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> -----------------------------------------------------------
> 
> (Updated May 8, 2018, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
>     https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/2/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/#review202691
-----------------------------------------------------------


Ship it!




Ship It!

- Andrei Budnik


On May 8, 2018, 8:36 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> -----------------------------------------------------------
> 
> (Updated May 8, 2018, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
>     https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/2/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/
-----------------------------------------------------------

(Updated May 8, 2018, 1:36 p.m.)


Review request for mesos, Andrei Budnik and James Peach.


Bugs: MESOS-8884
    https://issues.apache.org/jira/browse/MESOS-8884


Repository: mesos


Description
-------

This test was flaky because termination could already happened when we
set up the expectation. Given that we already verified task state, I do
not see checking container termination explicitly is necessary, so
removing the expectation should fix the flakiness.


Diffs (updated)
-----

  src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 


Diff: https://reviews.apache.org/r/66993/diff/2/

Changes: https://reviews.apache.org/r/66993/diff/1-2/


Testing
-------

GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose


Thanks,

Zhitao Li

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/#review202635
-----------------------------------------------------------




src/tests/containerizer/docker_containerizer_tests.cpp
Line 644 (original), 644 (patched)
<https://reviews.apache.org/r/66993/#comment284567>

    We can add
    ```cpp
      Future<Nothing> executorTerminated =
        FUTURE_DISPATCH(_, &Slave::executorTerminated);
    ```
    here and wait for it before stopping the driver
    ```cpp
    AWAIT_READY(executorTerminated);
    ```
    WDYT?


- Andrei Budnik


On May 7, 2018, 10:25 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> -----------------------------------------------------------
> 
> (Updated May 7, 2018, 10:25 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
>     https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/1/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66993/#review202612
-----------------------------------------------------------



Patch looks great!

Reviews applied: [66993]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 7, 2018, 10:25 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> -----------------------------------------------------------
> 
> (Updated May 7, 2018, 10:25 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
>     https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/1/
> 
> 
> Testing
> -------
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>