You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Yuval Pavel Zholkover <pa...@gmail.com> on 2013/10/22 00:14:49 UTC

Review Request 14800: glog creates log files in /tmp

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

Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs
-----

  src/logging/logging.cpp 850fb3c6f30afb0cb6a1f3a4a49abb87413cfb1d 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 22, 2013, 1:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?
> 
> Yuval Pavel Zholkover wrote:
>     Sorry, I'm learning to use reviewboard properly.
>     FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.
>     
>     Current mesos behaviour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
>     * without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     Propsed patch v2 behavour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
>     * without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.
> 
> Ben Mahler wrote:
>     Hey Yuval, is there an issue in the glog issue tracker related to the bug you described below?
> 
> Yuval Pavel Zholkover wrote:
>     Hi Ben, thanks for taking the time to review this. I did some digging and I think this is related but not the exact issue:
>     http://code.google.com/p/google-glog/issues/detail?id=128
>     
>

Perhaps to get around the extra verbosity we can do:

google::SetStderrLogging(3);

As suggested in the linked issue?


- Ben


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


On Oct. 22, 2013, 6:28 a.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 6:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 22, 2013, 1:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?
> 
> Yuval Pavel Zholkover wrote:
>     Sorry, I'm learning to use reviewboard properly.
>     FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.
>     
>     Current mesos behaviour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
>     * without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     Propsed patch v2 behavour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
>     * without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.

Hey Yuval, is there an issue in the glog issue tracker related to the bug you described below?


- Ben


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


On Oct. 22, 2013, 6:28 a.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 6:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.

> On Oct. 22, 2013, 3:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?
> 
> Yuval Pavel Zholkover wrote:
>     Sorry, I'm learning to use reviewboard properly.
>     FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.
>     
>     Current mesos behaviour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
>     * without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     Propsed patch v2 behavour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
>     * without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.
> 
> Ben Mahler wrote:
>     Hey Yuval, is there an issue in the glog issue tracker related to the bug you described below?

Hi Ben, thanks for taking the time to review this. I did some digging and I think this is related but not the exact issue:
http://code.google.com/p/google-glog/issues/detail?id=128


- Yuval Pavel


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


On Oct. 22, 2013, 8:28 a.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 8:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.

> On Oct. 22, 2013, 3:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?
> 
> Yuval Pavel Zholkover wrote:
>     Sorry, I'm learning to use reviewboard properly.
>     FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.
>     
>     Current mesos behaviour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
>     * without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     Propsed patch v2 behavour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
>     * without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.
> 
> Ben Mahler wrote:
>     Hey Yuval, is there an issue in the glog issue tracker related to the bug you described below?
> 
> Yuval Pavel Zholkover wrote:
>     Hi Ben, thanks for taking the time to review this. I did some digging and I think this is related but not the exact issue:
>     http://code.google.com/p/google-glog/issues/detail?id=128
>     
>
> 
> Ben Mahler wrote:
>     Perhaps to get around the extra verbosity we can do:
>     
>     google::SetStderrLogging(3);
>     
>     As suggested in the linked issue?
> 
> Yuval Pavel Zholkover wrote:
>     google::SetStderrLogging(3); just sets FLAGS_stderrthreshold. The issue is with the flow in LogMessage::SendToLog():
>       // global flag: never log to file if set.  Also -- don't log to a
>       // file if we haven't parsed the command line flags to get the
>       // program name.
>       if (FLAGS_logtostderr || !IsGoogleLoggingInitialized()) {
>         ColoredWriteToStderr(data_->severity_,
>                              data_->message_text_, data_->num_chars_to_log_);
>         ...
>       } else {
>         ...
>         LogDestination::MaybeLogToStderr(data_->severity_, data_->message_text_,
>                                          data_->num_chars_to_log_);
>     
>     LogDestination::MaybeLogToStderr checks the severity against FLAGS_stderrthreshold and then calls ColoredWriteToStderr.
>     
>     Btw I've also found this one: http://code.google.com/p/google-glog/issues/detail?id=47.
>     minlevel might work as a special case: log_dir not set and --quiet is set so just set minlevel=3.
>     Testing it now

Great! it works. I've updated the diff


- Yuval Pavel


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


On Oct. 23, 2013, 9:50 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.

> On Oct. 22, 2013, 3:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?
> 
> Yuval Pavel Zholkover wrote:
>     Sorry, I'm learning to use reviewboard properly.
>     FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.
>     
>     Current mesos behaviour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
>     * without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     Propsed patch v2 behavour is as follows:
>     * no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
>     * without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
>     * with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
>     * with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>
>     
>     I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.
> 
> Ben Mahler wrote:
>     Hey Yuval, is there an issue in the glog issue tracker related to the bug you described below?
> 
> Yuval Pavel Zholkover wrote:
>     Hi Ben, thanks for taking the time to review this. I did some digging and I think this is related but not the exact issue:
>     http://code.google.com/p/google-glog/issues/detail?id=128
>     
>
> 
> Ben Mahler wrote:
>     Perhaps to get around the extra verbosity we can do:
>     
>     google::SetStderrLogging(3);
>     
>     As suggested in the linked issue?

google::SetStderrLogging(3); just sets FLAGS_stderrthreshold. The issue is with the flow in LogMessage::SendToLog():
  // global flag: never log to file if set.  Also -- don't log to a
  // file if we haven't parsed the command line flags to get the
  // program name.
  if (FLAGS_logtostderr || !IsGoogleLoggingInitialized()) {
    ColoredWriteToStderr(data_->severity_,
                         data_->message_text_, data_->num_chars_to_log_);
    ...
  } else {
    ...
    LogDestination::MaybeLogToStderr(data_->severity_, data_->message_text_,
                                     data_->num_chars_to_log_);

LogDestination::MaybeLogToStderr checks the severity against FLAGS_stderrthreshold and then calls ColoredWriteToStderr.

Btw I've also found this one: http://code.google.com/p/google-glog/issues/detail?id=47.
minlevel might work as a special case: log_dir not set and --quiet is set so just set minlevel=3.
Testing it now


- Yuval Pavel


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


On Oct. 22, 2013, 8:28 a.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 8:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.

> On Oct. 22, 2013, 3:05 a.m., Benjamin Hindman wrote:
> > src/logging/logging.cpp, lines 104-106
> > <https://reviews.apache.org/r/14800/diff/1/?file=368956#file368956line104>
> >
> >     I think we want to add an else if which turns logging to stderr back off:
> >     
> >     if (!flags.quiet) {
> >       FLAGS_stderrthreshold = 0; // INFO.
> >     } else {
> >       FLAGS_logtostderr = false;
> >     }
> >     
> >     Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?

Sorry, I'm learning to use reviewboard properly.
FLAGS_logtostderr must always be set true whenever log_dir is empty. It actually prevents glog from writing files to /tmp.

Current mesos behaviour is as follows:
* no extra arguments - logs with severity level INFO and higher are written to stderr and to files in /tmp (this is wrong).
* without log_dir and --quiet - logs with severity level ERROR and higher are written to stderr and to files in /tmp (this is wrong).
* with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
* with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>

Propsed patch v2 behavour is as follows:
* no extra arguments - logs with severity level INFO and higher are written to stderr and no files are outputed.
* without log_dir and --quiet - logs with severity level INFO and higher (this is wrong) are written to stderr and no files are outputed.
* with log_dir set   - logs with severity level INFO and higher are written to stderr and to files in <logdir>.
* with log_dir set and --quiet - logs with severity level ERROR and higher are written to stderr and to files in <logdir>

I think the patch v2 behaviour makes more sense, as the only thing it does is being extra verbose when log_dir is not set and --quiet is requsted.


- Yuval Pavel


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


On Oct. 22, 2013, 8:28 a.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 8:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/#review27268
-----------------------------------------------------------



src/logging/logging.cpp
<https://reviews.apache.org/r/14800/#comment53116>

    Awesome! Thanks!



src/logging/logging.cpp
<https://reviews.apache.org/r/14800/#comment53117>

    I think we want to add an else if which turns logging to stderr back off:
    
    if (!flags.quiet) {
      FLAGS_stderrthreshold = 0; // INFO.
    } else {
      FLAGS_logtostderr = false;
    }
    
    Otherwise, if someone specifies 'quiet' they'll still get logging to stderr. Make sense?


- Benjamin Hindman


On Oct. 21, 2013, 10:14 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2013, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c6f30afb0cb6a1f3a4a49abb87413cfb1d 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 24, 2013, 8:24 p.m., Ben Mahler wrote:
> > Great work Yuval!

This is now committed, please mark it as submitted!


- Ben


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


On Oct. 23, 2013, 9:46 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/#review27483
-----------------------------------------------------------

Ship it!


Great work Yuval!

- Ben Mahler


On Oct. 23, 2013, 9:46 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 11:46 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Changes
-------

updated, PTAL


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs (updated)
-----

  src/logging/logging.cpp 850fb3c 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 23, 2013, 8:53 p.m., Ben Mahler wrote:
> > Thanks for following through with this Yuval!! Can you confirm again that now all the combinations of --log_dir and --quiet work as expected?
> 
> Yuval Pavel Zholkover wrote:
>     Yes I confirm that all 4 combinations of --log_dir and --quiet behave as expected. Sorry for the multiple updates; revisions r5 is just a whitespace cleanup, and r4 is a more concise version of r3.
>

Perfect, can you do a final cleanup of the logic based on my comments below? Then we're all set!


> On Oct. 23, 2013, 8:53 p.m., Ben Mahler wrote:
> > src/logging/logging.cpp, line 100
> > <https://reviews.apache.org/r/14800/diff/5/?file=369963#file369963line100>
> >
> >       ...
> >       // Do not log to stderr instead of log files.
> >       FLAGS_logtostderr = false;
> >     } else {
> >       // Log to stderr instead of log files.
> >       FLAGS_logtostderr = true;
> >     }


- Ben


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


On Oct. 23, 2013, 8:03 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 8:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.

> On Oct. 23, 2013, 10:53 p.m., Ben Mahler wrote:
> > Thanks for following through with this Yuval!! Can you confirm again that now all the combinations of --log_dir and --quiet work as expected?

Yes I confirm that all 4 combinations of --log_dir and --quiet behave as expected. Sorry for the multiple updates; revisions r5 is just a whitespace cleanup, and r4 is a more concise version of r3.


- Yuval Pavel


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


On Oct. 23, 2013, 10:03 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 10:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/#review27419
-----------------------------------------------------------


Thanks for following through with this Yuval!! Can you confirm again that now all the combinations of --log_dir and --quiet work as expected?


src/logging/logging.cpp
<https://reviews.apache.org/r/14800/#comment53280>

      ...
      // Do not log to stderr instead of log files.
      FLAGS_logtostderr = false;
    } else {
      // Log to stderr instead of log files.
      FLAGS_logtostderr = true;
    }



src/logging/logging.cpp
<https://reviews.apache.org/r/14800/#comment53281>

    if (flags.quiet) {
      FLAGS_stderrthreshold = 3; // FATAL.
    
      // FLAGS_stderrthreshold is ignored when logging to stderr instead of log files.
      // Setting the minimum log level gets around this issue.
      if (FLAGS_logtostderr) {
        FLAGS_minloglevel = 3; // FATAL.
      }
    } else {
      FLAGS_stderrthreshold = 0; // INFO.
    }


- Ben Mahler


On Oct. 23, 2013, 8:03 p.m., Yuval Pavel Zholkover wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14800/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 8:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
> Setting FLAGS_logtostderr to true disables all log file creation/writes.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.cpp 850fb3c 
> 
> Diff: https://reviews.apache.org/r/14800/diff/
> 
> 
> Testing
> -------
> 
> manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
> the patch fixes it
> 
> 
> Thanks,
> 
> Yuval Pavel Zholkover
> 
>


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 10:03 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs (updated)
-----

  src/logging/logging.cpp 850fb3c 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 10:01 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs (updated)
-----

  src/logging/logging.cpp 850fb3c 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 9:50 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs (updated)
-----

  src/logging/logging.cpp 850fb3c 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover


Re: Review Request 14800: glog creates log files in /tmp

Posted by Yuval Pavel Zholkover <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14800/
-----------------------------------------------------------

(Updated Oct. 22, 2013, 8:28 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Changes
-------

I've updated the patch unfortunately, flags.quiet will not work when log_dir is not set.
It looks like a bug in glog void LogMessage::SendToLog()

if (FLAGS_logtostderr || !IsGoogleLoggingInitialized()) {
    ColoredWriteToStderr(data_->severity_,
                         data_->message_text_, data_->num_chars_to_log_);

ColoredWriteToStderr does not respect the severity threshold, it should have been

if (FLAGS_logtostderr || !IsGoogleLoggingInitialized()) {
    LogDestination::MaybeLogToStderr(data_->severity_, data_->message_text_,
                                     data_->num_chars_to_log_);


Repository: mesos-git


Description
-------

The current intialization of glog creates log files in /tmp even when the log_dir command argument is not passed.
Setting FLAGS_logtostderr to true disables all log file creation/writes.


Diffs (updated)
-----

  src/logging/logging.cpp 850fb3c 

Diff: https://reviews.apache.org/r/14800/diff/


Testing
-------

manual - during make check /tmp gets filled with glog logs (timestamped and symlinked)
the patch fixes it


Thanks,

Yuval Pavel Zholkover