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

Ship it!


Only high-level comment is that the _container_ logger talks a lot about 
executor's and tasks, I need to look into some of the upcoming reviews, but why 
isn't it sufficient to only talk about containers?


include/mesos/slave/container_logger.hpp (line 40)
<https://reviews.apache.org/r/41002/#comment171027>

    s/executors/containers/
    
    s/of tasks launched by default executors/of tasks launched without an 
executor (that implicitly use the command executor)/



include/mesos/slave/container_logger.hpp (lines 43 - 44)
<https://reviews.apache.org/r/41002/#comment171028>

    Make this part a TODO?



include/mesos/slave/container_logger.hpp (lines 73 - 75)
<https://reviews.apache.org/r/41002/#comment171029>

    Can we document what the failure case of using an IO::PIPE means? As in, 
what if someone specifies an IO::PIPE? What breaks down?



include/mesos/slave/container_logger.hpp (line 125)
<https://reviews.apache.org/r/41002/#comment171030>

    s/Error/Failure/
    s/error/failure/



include/mesos/slave/container_logger.hpp (line 128)
<https://reviews.apache.org/r/41002/#comment171031>

    What's the long term plan for this? Will we have multiple container loggers 
running simultaneously at some point?
    
    I'd like you to capture a TODO here to follow up with the semantics on 
executors that can't be recovered. And let's roll this into the next phase so 
that we have consistent semantics here.



include/mesos/slave/container_logger.hpp (line 135)
<https://reviews.apache.org/r/41002/#comment171032>

    s/launches an executor or a default executor launches a task/creates a 
container/



include/mesos/slave/container_logger.hpp (line 139)
<https://reviews.apache.org/r/41002/#comment171033>

    s/an executor or task/container/



include/mesos/slave/container_logger.hpp (line 155)
<https://reviews.apache.org/r/41002/#comment170823>

    Kill newline.


- Benjamin Hindman


On Dec. 15, 2015, 8:38 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41002/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 8:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, Artem Harutyunyan, 
> and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-4087
>     https://issues.apache.org/jira/browse/MESOS-4087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The container logger is an agent module whose job is to take an executor or 
> task's stdout/stderr, and pipe it to some sink.
> 
> Existing executor and task stdout/stderr are piped into files ("stdout" and 
> "stderr") located in the executor's sandbox directory.
> This module aims to encompass this default behavior as well as allow more 
> sophisticated logging solutions.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/container_logger.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41002/diff/
> 
> 
> Testing
> -------
> 
> This is added to the Makefile later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to