mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 50422: Future-proofed some slave removal tests.
Date Thu, 28 Jul 2016 10:17:36 GMT


> On July 27, 2016, 11:54 p.m., Vinod Kone wrote:
> > src/tests/slave_tests.cpp, line 2428
> > <https://reviews.apache.org/r/50422/diff/1/?file=1452094#file1452094line2428>
> >
> >     Are you sure pausing the clock at the beginning is safe? IIRC, the scheduler
driver does reliable registration which relies on timers and with esp auth enabled there is
no guarantee that auth and registration go through in the first try?
> >     
> >     Also, please run these tests in a loop if you haven't already.

Hmmm -- we pause the clock as the first step of a lot of tests, so if it isn't safe that is
a bigger issue. I do think pausing the clock for the entirety of the test has some advantages
in terms of code clarity (and I'd like to see us consider running all tests with the clock
paused by default). Anyway, I reverted this change for now.


- Neil


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


On July 28, 2016, 10:14 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50422/
> -----------------------------------------------------------
> 
> (Updated July 28, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> These tests relied on the implementation detail that when an agent is
> removed from the list of registered agents, the master sends a
> ShutdownSlaveMessage to the agent. That will change in the future
> (MESOS-4049). To prepare for this future planned behavior, adjust these
> tests to be more robust by instead checking for the invocation of the
> `slaveLost` scheduler callback.
> 
> 
> Diffs
> -----
> 
>   src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 
> 
> Diff: https://reviews.apache.org/r/50422/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Validated that when the "cancel pending slave removal on receipt of ping" code is removed,
`CancelSlaveRemoval` still fails.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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