----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41783/#review113378 -----------------------------------------------------------
src/slave/container_loggers/rotate.hpp (line 36) <https://reviews.apache.org/r/41783/#comment173976> { on newline please. src/slave/container_loggers/rotate.hpp (lines 46 - 50) <https://reviews.apache.org/r/41783/#comment173977> This should be `Bytes` rather than force the reader/user to write this in bytes (even if it's code that is generating this) and force you to use it as a `size_t`. Same for other flags you have here and below too please. src/slave/container_loggers/rotate.hpp (line 54) <https://reviews.apache.org/r/41783/#comment173979> How about s/of log/of rotated log/ here? Also, is 'head' better than 'current'? Just a suggestion, given that head sounds like "first", I thought by head you meant the oldest log file, not the newest. src/slave/container_loggers/rotate.cpp (lines 35 - 37) <https://reviews.apache.org/r/41783/#comment174225> These pulled out? src/slave/container_loggers/rotate.cpp (lines 57 - 61) <https://reviews.apache.org/r/41783/#comment174226> Can you send me another review which updates the version of `io::read` that you're using to `dup`, `os::cloexec`, and `os::nonblock` the file descriptor so we don't have to? Then we can kill this (and other places that use that version of `io::read` too!). src/slave/container_loggers/rotate.cpp (lines 207 - 218) <https://reviews.apache.org/r/41783/#comment175067> Want to do this validation in the flags themselves? src/slave/container_loggers/rotating.hpp (line 49) <https://reviews.apache.org/r/41783/#comment173975> How about: s/of stdout/of rotated stdout (i.e., stdout.1, stdout.2, ...)/ - Benjamin Hindman On Jan. 8, 2016, 8:06 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41783/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2016, 8:06 p.m.) > > > Review request for mesos, Benjamin Hindman and Artem Harutyunyan. > > > Bugs: MESOS-4136 > https://issues.apache.org/jira/browse/MESOS-4136 > > > Repository: mesos > > > Description > ------- > > Adds a non-default ContainerLogger that constrains total log size by rotating > logs (i.e. renaming the head log file). > > > Diffs > ----- > > src/slave/container_loggers/rotate.hpp PRE-CREATION > src/slave/container_loggers/rotate.cpp PRE-CREATION > src/slave/container_loggers/rotating.hpp PRE-CREATION > src/slave/container_loggers/rotating.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/41783/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Joseph Wu > >
