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 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.
Date Thu, 17 Dec 2015 23:54:01 GMT


> On Dec. 17, 2015, 12:56 p.m., Benjamin Hindman wrote:
> > 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?

I wanted to be explicit about the scope of the module.  i.e. All executors/tasks run in containers,
but not all containers are executors/tasks.

Changed to use "container" in the comments.


> On Dec. 17, 2015, 12:56 p.m., Benjamin Hindman wrote:
> > include/mesos/slave/container_logger.hpp, lines 43-44
> > <https://reviews.apache.org/r/41002/diff/8/?file=1165100#file1165100line43>
> >
> >     Make this part a TODO?

I'll add a TODO to expose some sort of message or URL in the Mesos web UI.  

The module still needs to provide a log-reading interface (because write-only logs aren't
very useful).  But Mesos won't know how to read said logs.


> On Dec. 17, 2015, 12:56 p.m., Benjamin Hindman wrote:
> > include/mesos/slave/container_logger.hpp, lines 73-75
> > <https://reviews.apache.org/r/41002/diff/8/?file=1165100#file1165100line73>
> >
> >     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?

Would it be reasonable to `ABORT` the agent if the module specifies a `PIPE`?  Or perhaps
make this a compile-time restriction (i.e. by introducing a subclass of `Subprocess::IO`).

(Note: The check for `PIPE` isn't implemented yet.)


> On Dec. 17, 2015, 12:56 p.m., Benjamin Hindman wrote:
> > include/mesos/slave/container_logger.hpp, line 128
> > <https://reviews.apache.org/r/41002/diff/8/?file=1165100#file1165100line128>
> >
> >     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.

For Docker command executors, there will be a container logger running alongside the executor.
 But it will only ever spawn one task (because the executor will refuse >1 task).

In general, this means we could:

- Launch an executor that logs to the sandbox,
- Restart the agent with a special container logger (say, to syslog),
- Have the recovered executor continue logging to the sandbox, while new executors log to
syslog.


- Joseph


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


On Dec. 17, 2015, 3:46 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41002/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 3:46 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
> 
>


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