mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 52762: Removed 'recover()' interface in 'ContainerLogger'.
Date Wed, 12 Oct 2016 01:53:20 GMT

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


Ship it!




I'm going to take some liberties with your commit message :)

For the most part, you don't need to describe the two parameters in this commit.  That description
is more suited for: https://reviews.apache.org/r/52412/

- Joseph Wu


On Oct. 11, 2016, 5:09 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52762/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Joris Van Remoortere, and Joseph
Wu.
> 
> 
> Bugs: MESOS-6371
>     https://issues.apache.org/jira/browse/MESOS-6371
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change arises from the nested container support in Mesos.
> 
> Currently, the container logger interface mainly contains
> `recover()` and `prepare()` methods. The `prepare` will be
> called in containerizer::launch() to launch a container,
> while `recover` will be called in containerizer::recover()
> to recover containers. Both methods rely on 2 parameters:
> ExecutorInfo and sandbox directory. The sandbox directory
> for nested containers can still be passed to the logger.
> However, because of nested container support, ExecutorInfo
> is no longer available for nested containers.
> 
> In logger prepare, the ExecutorInfo is used for deliver
> FrameworkID, ExecutorID, and Label for custom metadata.
> In containerizer launch, we can still pass the ExecutorInfo
> of a nested container's top level parent to the logger,
> so that those information will not be lost.
> 
> In logger recover, since currently the logger is stateless,
> and most of the logger modules are doing `noop` in
> logger::recover(). The recover interface should exist
> together with `cleanup` method if the logger become stateful
> in the future. To avoid adding tech debt in containerizer
> nested container support, we should remove the `recover`
> in container logger for now (can add it back together with
> `cleanup` in the future if the container logger become
> stateful).
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/container_logger.hpp 9623b0cd2785c4581180bffd51cd74db81146f1e 
>   src/slave/container_loggers/lib_logrotate.hpp a8653d716a898f03cce83f7b88b666dc46c0ea90

>   src/slave/container_loggers/lib_logrotate.cpp 0ca2b3d7dbb57c11c0740aed3914a6b75329af99

>   src/slave/container_loggers/sandbox.hpp 8b1f8ab6ce92ef4a2594bbae9269d42665ca1475 
>   src/slave/container_loggers/sandbox.cpp 807d79205a4e649ec4dfa0b130a549a7f312f0ec 
>   src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 
>   src/slave/containerizer/mesos/containerizer.cpp 32058c35ea9ca95f0a2665994c1ebccd5c840345

>   src/tests/container_logger_tests.cpp f76117230e0517ddc3cb8e0bf482085fad6950d2 
> 
> Diff: https://reviews.apache.org/r/52762/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


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