> On March 25, 2014, 12:23 a.m., Ben Mahler wrote: > > src/master/master.cpp, lines 514-519 > > <https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514> > > > > Would it not be simpler to disallow FATAL-only logging for now so that > > we don't have to do this special casing in the Master / Slave / webui? > > > > Or, if we want to keep FATAL-only logging as an option, we could update > > the "pailer" component of the webui show the underlying error from the > > backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts? > > Alexandra Sava wrote: > When a FATAL log is been issued, the program exits(this is how glog > works) so you won't be able to access the log file from the web gui. I > already updated the webgui for the case when minloglevel is FATAL (in this > patch), so you will not see "FAILED TO INITIALIZE...RETRYING". > > Ben Mahler wrote: > I understand the semantics of FATAL logging and the log file creation, > I'm just not convinced we should add the special case logic in the Master, > Slave, webui, just to support FATAL-only logging. So, I'm wondering why we > don't just support INFO,WARNING,ERROR to keep things simple. If it turns out > that people need FATAL we can add it. > > Alexandra Sava wrote: > If FATAL logs are used int the code, why not having the possibility of > configuring the log level as FATAL? I don't see where it gets complicated if > we have a few lines more for FATAL logs.
We had to change the master, slave, logging library, and the webui to handle the FATAL case. In general, special cases are unfortunate because it means an "exceptional" case that we need to: 1. Remember in our mental model of the system. 2. Add comments to inform the reader of the code. 3. Handle correctly in all the appropriate places (opening room for more bugs). This is why I feel supporting FATAL introduces too much complexity for the use-case. Does this make sense? As vinod said, following up in a separate review would be great! - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19357/#review38382 ----------------------------------------------------------- On April 28, 2014, 7:14 a.m., Alexandra Sava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19357/ > ----------------------------------------------------------- > > (Updated April 28, 2014, 7:14 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-1067 > https://issues.apache.org/jira/browse/MESOS-1067 > > > Repository: mesos-git > > > Description > ------- > > minloglevel flag is passed as an argument to the command line and it > configures the level of logging to stderr and file. It can take INFO, > WARNING, ERROR or FATAL values. All log messages > at or above the configured log level will be printed. > > If 'quiet' flag is also passed to the command line, minloglevel will > affect just the logs from file (if configured by log_dir flag) > > If a log directory is present, the log file that will be available for > viewing in the browser > will be the one for the minimum log level configured (with or without the > presence of minloglevel flag) > If FATAL is the minimum log level, there is an exception: no log file will > be available in the browser. This is because FATAL logs will cause the > program to crash. > So if at any point the user can still access mesos page in the browser, it > means no > FATAL logs have been issued. > > > Diffs > ----- > > src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6 > src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7 > src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a > src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb > src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4 > src/webui/master/static/js/controllers.js > 4b8487e0c285f892ad352993c81637f38df1429f > > Diff: https://reviews.apache.org/r/19357/diff/ > > > Testing > ------- > > 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag > set to WARNING > - expected results: - logs at and above WARNING level are logged into > log_dir and stderr > - the log file for WARNING logs is accessible in > the browser > > 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag > set to FATAL > - expected results: - logs at FATAL level are logged into log_dir and > stderr > - no log file is accessible in the browser > because as soon as a FATAL log will be issued, the program will crash > > 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag > set to INFO and quiet flag defined > - expected results: - logs at and above INFO level are logged into > log_dir and no logs (just FATAL) are displayed at stderr > - the log file for INFO logs is accessible in the > browser > > 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet > flag defined > - expected results: - no logs (just FATAL) are displayed at stderr > (quiet flag takes precedence) > - no log file is accessible in the browser > > 5.Test5 - run master and slave with log_dir and quiet flags defined > - expected results: - no logs (just FATAL) are displayed at stderr > - the log file for INFO logs is accessible in the > browse > > > Thanks, > > Alexandra Sava > >
