mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 69436: Fixed flaky check in cluster::Slave destructor.
Date Mon, 26 Nov 2018 17:20:16 GMT


> On Nov. 23, 2018, 1:44 p.m., Andrei Budnik wrote:
> > src/tests/cluster.cpp
> > Lines 699 (patched)
> > <https://reviews.apache.org/r/69436/diff/1/?file=2110098#file2110098line699>
> >
> >     What if there is more than one container whose `Future<ContainerTermination>`
is ready? Should we call `settle()` for each of these containers?
> 
> Benno Evers wrote:
>     At this point we know from the `foreach`-loop above that all `Future<ContainerTermination>`s
are ready, so all calls to `defer()` in `composing.cpp` should have already happened. So I
think a single call to settle should guarantee that all of them will be processed by libprocess
before the clock is settled.

Can we check this assumption by trying to launch say 1000 containers in the given test?


- Andrei


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


On Nov. 22, 2018, 7:43 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69436/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2018, 7:43 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Joseph Wu.
> 
> 
> Bugs: MESOS-9272
>     https://issues.apache.org/jira/browse/MESOS-9272
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The destructor of `cluster::Slave` contained an assertion
> that was not safe to assume in the presence of the
> composing containerizer. This commit adds an additional
> `Clock::settle()` to fix the issue.
> 
> 
> Diffs
> -----
> 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69436/diff/1/
> 
> 
> Testing
> -------
> 
> Ran `./src/mesos-tests --gtest_filter="SlaveTest.DefaultExecutorCommandInfo" --verbose
--gtest_repeat=50000 --gtest_throw_on_failure` while simulatenously running `stress-ng --random
64` on the same machine.
> 
> (before the change, `SlaveTest.DefaultExecutorCommandInfo` would fail roughly once every
15000 runs without `stress-ng` and roughly once every 300 runs with `stress-ng`)
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message