mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nishant Suneja" <nishantsun...@gmail.com>
Subject Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully
Date Sun, 28 Dec 2014 07:29:24 GMT


> On Dec. 26, 2014, 9:11 p.m., Timothy Chen wrote:
> > Can you add a unit test?
> 
> Nishant Suneja wrote:
>     I wanted to add one, but I would need access to the "Future" object for the container
launch, or "Timer" object for registration timeout trigger, both of which
>     I cannot access, since they are local variables, and NOT the member variables of
any class.
>     
>     Any ideas as to how do I get around it, without introducting any extra member variables
?
> 
> Timothy Chen wrote:
>     you can get hold of the future by passing in a mock containerizzer and perform a
EXPECT_CALL(comtainerizer, launch(.....)) and return a future that you instantiate in test.
> 
> Nishant Suneja wrote:
>     Thats a neat way to get hold of the future object. So, for testing I have to basically
verify that the timer object for Slave's "registerExecutor" is not created, till we reach
the READY state of the future object. Now, I could have done something crazy like iterate
the map of timer objects in the Clock class, and verify that we dont have this timer object
till we are in READY state of the future object.
>     However, this map is a defined as static in clock.cpp, so I wouldnt be able to access
it in the testcase, without making the map global (not a good idea).
>     
>     So, again, any ideas ?
> 
> Timothy Chen wrote:
>     The easiest way is to simply advance the clock (Clock::advance(....)) until the timeout,
and verify that the delay call is not yet called. Then you satisfy the future and advance
the time again and verify it is called.

Ok...And whats the best way to verify that the "Slave::registerExecutorTimeout" has been called
(after advancing the clock). I could check the state of the executor, and verify that its
TERMINATING, which would mean that this method has executed.

Any other cleaner way to verify this callback ? I believe that EXPECT_CALL macro only works
for classes with MOCK_METHOD set of methods defined. So, I cant use that.


- Nishant


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


On Dec. 26, 2014, 6:29 p.m., Nishant Suneja wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29437/
> -----------------------------------------------------------
> 
> (Updated Dec. 26, 2014, 6:29 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-999
>     https://issues.apache.org/jira/browse/MESOS-999
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> As part of this bug fix, I have trigerred the executor registration timeout timer asychronously,
> when the onReady() callback is made for the container's future object, instead of starting
the
> timer synchronously when the launchExecutor() method of the Framework class is invoked.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b 
> 
> Diff: https://reviews.apache.org/r/29437/diff/
> 
> 
> Testing
> -------
> 
> make check succeeds.
> 
> 
> Thanks,
> 
> Nishant Suneja
> 
>


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