You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2017/05/22 18:35:19 UTC
Review Request 59460: Added new agent flag
'executor_reregister_timeout'.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59460/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
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
Re: Review Request 59460: Added new agent flag
'executor_reregister_timeout'.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59460/#review175707
-----------------------------------------------------------
src/slave/flags.cpp
Lines 335 (patched)
<https://reviews.apache.org/r/59460/#comment249049>
s/reregister/reregistration/
to be consistent with the above flag.
src/slave/flags.cpp
Lines 340 (patched)
<https://reviews.apache.org/r/59460/#comment249048>
s/offers./offers after an agent restart./
src/slave/flags.cpp
Lines 345 (patched)
<https://reviews.apache.org/r/59460/#comment249050>
s/to be less than/to be not more than/ ?
src/slave/slave.cpp
Lines 4204 (patched)
<https://reviews.apache.org/r/59460/#comment249051>
re-registration
- Vinod Kone
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
>
>
Re: Review Request 59460: Added new agent flag
'executor_reregister_timeout'.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 59460: Added new agent flag
'executor_reregistration_timeout'.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59460/
-----------------------------------------------------------
(Updated May 22, 2017, 10:36 p.m.)
Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
Summary (updated)
-----------------
Added new agent flag 'executor_reregistration_timeout'.
Bugs: MESOS-7540
https://issues.apache.org/jira/browse/MESOS-7540
Repository: mesos
Description (updated)
-------
This patch adds a new agent flag, 'executor_reregistration_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
-----
docs/configuration.md c5194e7566c13514f55742aa7278e75314f69d9c
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/2/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 59460: Added new agent flag
'executor_reregister_timeout'.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59460/
-----------------------------------------------------------
(Updated May 22, 2017, 10:36 p.m.)
Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
Changes
-------
Addressed comments, added docs.
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 (updated)
-----
docs/configuration.md c5194e7566c13514f55742aa7278e75314f69d9c
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/2/
Changes: https://reviews.apache.org/r/59460/diff/1-2/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 59460: Added new agent flag
'executor_reregister_timeout'.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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