mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 55612: Fixed SlaveRecoveryTest.RecoverTerminatedExecutor test.
Date Wed, 18 Jan 2017 12:38:49 GMT


> On Jan. 18, 2017, 7:24 a.m., Greg Mann wrote:
> > It seems like a bit of a shame that the standard agent recovery path involves two
calls to `statusUpdateManager->resume`, which are likely to occur in quick succession.
However, it seems like we expect frameworks to handle duplicate status updates gracefully,
so perhaps it's not a big deal. It's not clear to me yet from looking at the code if there
would be an easy way to avoid the duplicates in this case.

Agreed, it's unfortunate. Spent sometime thinking if there is an easy way to fix it, but couldn't
come up with one.


> On Jan. 18, 2017, 7:24 a.m., Greg Mann wrote:
> > src/tests/slave_recovery_tests.cpp, line 1204
> > <https://reviews.apache.org/r/55612/diff/2/?file=1606525#file1606525line1204>
> >
> >     As long as you're cleaning this up, might as well fix the spacing - should be
just one space before the comment begins.

Do you mean the spacing between "Return());" and "//" ? Quite a few of such comments in this
file use more than one space for this case.


- Vinod


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


On Jan. 17, 2017, 1:06 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55612/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 1:06 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-6922
>     https://issues.apache.org/jira/browse/MESOS-6922
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Just like RecoverHTTPTerminatedExecutor test, the scheduler should
> handle duplicate status updates after agent restarts. See the
> attached ticket for details.
> 
> 
> Diffs
> -----
> 
>   src/tests/slave_recovery_tests.cpp 9323cbb74dd555fe6c44af1ef6da70e0c2d76dcb 
> 
> Diff: https://reviews.apache.org/r/55612/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Ran in a loop for 20 times.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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