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


Fix it, then Ship it!





src/slave/constants.hpp
Line 36 (original), 36 (patched)
<https://reviews.apache.org/r/59460/#comment249067>

    You'll also want to rename this one to be consistent with the flag name.



src/slave/constants.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/59460/#comment249055>

    How about 15 seconds here and we can increase if needed? In particular I'm 
a little concerned about this being high, since the agent has to wait for this 
entire timeout if an executor is dead.



src/slave/flags.hpp
Line 79 (original), 79-80 (patched)
<https://reviews.apache.org/r/59460/#comment249063>

    Name these two consistently?



src/slave/flags.cpp
Lines 337-338 (patched)
<https://reviews.apache.org/r/59460/#comment249056>

    Maybe mention that it's in the case of an agent restart? E.g. "re-register 
with after the agent has restarted, before the agent considers it gone and 
shuts it down"



src/slave/flags.cpp
Lines 338-340 (patched)
<https://reviews.apache.org/r/59460/#comment249058>

    It would be good to mention that this is the current implementation (since 
it could be better): "Note that the current implementation of agent recovery 
..." And maybe even include the ticket number in the help message for not 
always waiting, and detecting dead executors.
    
    Don't know if you need the "which will delay ..." since there's more we can 
say, e.g. "can't run tasks, can't send framework messages, etc".


- Benjamin Mahler


On May 22, 2017, 6:46 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59460/
> -----------------------------------------------------------
> 
> (Updated May 22, 2017, 6:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7540
>     https://issues.apache.org/jira/browse/MESOS-7540
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds a new agent flag, 'executor_reregister_timeout',
> which allows operators to configure the time which an executor has
> to reregister with the agent before it will be considered gone and
> terminated.
> 
> 
> Diffs
> -----
> 
>   src/slave/constants.hpp 9c1d7245c0a8ecd657bb64dafb8b0fa4780325f4 
>   src/slave/flags.hpp 28f6482c38a2d388e624726d5e7bccc5cb260fce 
>   src/slave/flags.cpp e172aa526cfd38f4f30efda22ef011146e5c1a7d 
>   src/slave/slave.cpp 14de72fa4a8746504a251d735bf56041b934df40 
> 
> 
> Diff: https://reviews.apache.org/r/59460/diff/1/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to