mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 59460: Added new agent flag 'executor_reregister_timeout'.
Date Mon, 22 May 2017 22:03:59 GMT

-----------------------------------------------------------
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
> 
>


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