mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@apache.org>
Subject Re: Review Request 65449: Fixed an issue where executor info linger on master if failed to launch.
Date Thu, 08 Feb 2018 20:41:28 GMT

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



Description: s/linger/lingers/


src/slave/slave.cpp
Lines 1816 (patched)
<https://reviews.apache.org/r/65449/#comment277226>

    `None()` on next line please



src/slave/slave.cpp
Line 2104 (original), 2127 (patched)
<https://reviews.apache.org/r/65449/#comment277227>

    don't need to send here?



src/slave/slave.cpp
Line 2543 (original), 2626 (patched)
<https://reviews.apache.org/r/65449/#comment277228>

    TASK_LOST or ExitedExecutorMessage



src/slave/slave.cpp
Lines 2671 (patched)
<https://reviews.apache.org/r/65449/#comment277229>

    for a command task `executor` should be null. Can you do a CHECK instead?



src/slave/slave.cpp
Lines 2682 (patched)
<https://reviews.apache.org/r/65449/#comment277230>

    s/We will drop tasks/In this case we will drop tasks/



src/slave/slave.cpp
Lines 2715 (patched)
<https://reviews.apache.org/r/65449/#comment277231>

    should we send this only if executor state is TERMINATED? If the executor is in any other
state, we are shutting it down below which should result in a ExitedExecutorMessage?
    
    Also, can you add a TODO to have master do proper reconciliation instead of removing the
executor entry in this case?



src/slave/slave.cpp
Lines 2725 (patched)
<https://reviews.apache.org/r/65449/#comment277232>

    s/And the master will no longer contain/Master then removes/


- Vinod Kone


On Feb. 5, 2018, 3 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65449/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2018, 3 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
>     https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Master relies on `ExitedExecutorMessage` from the agent to recycle
> executor entry. However, this message won't be sent if the executor
> never actually launched (due to transient error), leaving executor
> info on the master lingering and resource claimed.
> See MESOS-1720.
> 
> This patch fixes this issue by sending the `ExitedExecutorMessage`
> from the agent if the executor is never launched.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
>   src/tests/mock_slave.hpp 29ce7140501888d95d5f2d6c26b752ad276b484a 
>   src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65449/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> Dedicated test in #65448
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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