mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 70595: Adds a regression test for MESOS-9766.
Date Mon, 06 May 2019 21:46:56 GMT

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


Fix it, then Ship it!




Thanks for making this test more deterministic! Got some comments, but IMO this test is robust
enough in practice so please feel free to drop my issues.


3rdparty/libprocess/src/tests/process_tests.cpp
Lines 2122 (patched)
<https://reviews.apache.org/r/70595/#comment301475>

    If you want libprocess to manager this actor, you can do the following:
    ```
    PID<TestProcess> pid = spawn(new TestProcess(), true);
    
    Future<Nothing> waited = dispatch(pid, &TestProcess::wait_for_terminate);
    ```
    Then you won't need to do `process::wait` and `delete` later.
    
    If you do so, you don't even need `waited`.



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 2138 (patched)
<https://reviews.apache.org/r/70595/#comment301474>

    Although highly unlikely, it is theoratically possible that the terminate event is enqueued
before any event of `process` got dequeued. If this happen, this await would fail even w/
the fix.
    
    A possible way to fix this is to set up some promise in `wait_for_terminate`, and then
wait for the corresponding future before calling `terminate` in this test.
    
    That said, fixing this flake that's unlikely to happen might not worth it, so please feel
free to drop this issue.


- Chun-Hung Hsiao


On May 6, 2019, 6:34 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70595/
> -----------------------------------------------------------
> 
> (Updated May 6, 2019, 6:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9766
>     https://issues.apache.org/jira/browse/MESOS-9766
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test fails on master prior to applying the fix for MESOS-9766.
> It attempts to ensure that processes are terminated after the
> /__processes__ handler dispatches to them.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 60f3dd653153c2b2ccf9c3a7eae8c75fd6ff025c

> 
> 
> Diff: https://reviews.apache.org/r/70595/diff/2/
> 
> 
> Testing
> -------
> 
> Ran in repetition, although it appears to consistently fail on master without repetition
needed.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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