mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 69681: Fixed the FD leak if containerizer::_launch() failed or discarded.
Date Mon, 07 Jan 2019 15:37:27 GMT

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 1375 (patched)
<https://reviews.apache.org/r/69681/#comment297288>

    This will *copy* the value of `_containerIO`, hence it is *always* empty.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1377 (patched)
<https://reviews.apache.org/r/69681/#comment297289>

    This might fail in case of discard is called *before* calling `.then` handler for `prepare()`.
    
    So, it should be better to:
    ```
    if (_containerIO.isSome()) {
      closeContainerIoFds(_containerIO.get());
    }
    ```



src/slave/containerizer/mesos/containerizer.cpp
Lines 1378 (patched)
<https://reviews.apache.org/r/69681/#comment297290>

    Will this lead to problem with a double `close` of the same FDs? We close FDs here, but
these FDs will be closed in destructor when the IOSB terminates and then removes the same
IO item from its hashmap.


- Andrei Budnik


On Jan. 7, 2019, 12:22 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69681/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2019, 12:22 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Jie Yu, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6632
>     https://issues.apache.org/jira/browse/MESOS-6632
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> For the period between IOSB server is up and container process exec,
> if the containerizer launch returns failure or discarded, the FD used
> for signaling from the container process to the IOSB finish redirect
> will be leaked, which would cause the IOSB stuck at io::redirect
> forever. It would block the containerizer from cleaning up this
> container at IOSB.
> 
> This issue could be exposed if there are frequent check containers
> launch on an agent with heavy loads.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.hpp c34f1296586ea5d26619605f8f6a5ae9444f1a96 
>   src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b

>   src/slave/containerizer/mesos/io/switchboard.cpp c445a8f09d7671d5763281bac9881489b3cc9c39

>   src/slave/containerizer/mesos/utils.hpp bfd07e28c78fc140e395ffccd11d65545bf007fc 
>   src/slave/containerizer/mesos/utils.cpp d9964f0b0e8ad8a41e5f5490a1c28bba9a972ae0 
> 
> 
> Diff: https://reviews.apache.org/r/69681/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


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