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