mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.
Date Mon, 29 Oct 2018 22:20:59 GMT


> On Oct. 29, 2018, 3:10 p.m., Meng Zhu wrote:
> > src/tests/gc_tests.cpp
> > Lines 919 (patched)
> > <https://reviews.apache.org/r/69203/diff/1/?file=2103054#file2103054line920>
> >
> >     NIT: ideally we should do `process::ID::generate()`.

Good point.  There are several other tests that support executor reregistration in this way,
and they all use the same ID.  (I won't change all of those right now.)  Generating the ID
would at least prevent this test from interfering with those tests in case the agent actor
leaks.


- Joseph


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


On Oct. 29, 2018, 12:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69203/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2018, 12:16 p.m.)
> 
> 
> Review request for mesos, Meng Zhu and Qian Zhang.
> 
> 
> Bugs: MESOS-9217
>     https://issues.apache.org/jira/browse/MESOS-9217
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test was incorrectly restarting the agent actor inside the test.
> In the flaky test, the agent actor would be started with an auto-
> generated PID (i.e. `slave(1)`, `slave(2)`, etc).  Because of how this
> generation works, each PID will be unique.  The executor in the test
> would be launched under `slave(1)` but the restarted agent would have
> a PID of `slave(2)`.  This meant the executor's reregistration would
> fail with '404 Not Found' and the executor would be cleaned up.
> 
> The executor cleanup would potentially trigger a TASK_LOST status
> update; and if that update is sent prior to ending the test, this
> will break some mock expectations and cause the test to fail.
> 
> This changes the test to always use the same PID for the agent actor.
> 
> 
> Diffs
> -----
> 
>   src/tests/gc_tests.cpp 4d94430b9de57c20f0f7fe7001a543dbf3a56f1d 
> 
> 
> Diff: https://reviews.apache.org/r/69203/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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