> On Feb. 18, 2022, 10:58 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/webapp/WEB-INF/logback.xml > > Lines 32 (patched) > > <https://reviews.apache.org/r/73856/diff/1/?file=2266415#file2266415line32> > > > > Is it necessary to remove log files from previous run on Ranger admin > > start (cleanHistoryOnStart=true)? Other than in dev environments, it will > > be desirable to persist logs across application restarts. > > Kirby Zhou wrote: > cleanHistoryOnStart=true just clean old files which exceed the limit of > maxHistory. it will not clean all. > In production environments, we can not have unlimited log files. > > In EmbeddedServer.java, we set max day of access-log to 15 by > > ``` > > valve.setMaxDays(EmbeddedServerUtil.getIntConfig(ACCESS_LOG_ROTATE_MAX_DAYS,15)); > ``` > > I think other logs have no longer retention value than access-log.
ok. Thanks for the details. Changes look good. > On Feb. 18, 2022, 10:58 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/webapp/WEB-INF/logback.xml > > Line 78 (original), 88 (patched) > > <https://reviews.apache.org/r/73856/diff/1/?file=2266415#file2266415line88> > > > > For perf appender, the level should be debug. Please retain existing > > value. > > Kirby Zhou wrote: > You mean perf appender DEBUG should be enabled by default to final user? > It is too noisy, I think it should not be enabled by default. > Developers can enabled it by hand. > > On the other hand, we should include timecost in the access log by > default. > > Add a "%D" in EmbeddedServer.java here > > ``` > - String defaultAccessLogPattern = > servername.equalsIgnoreCase(KMS_SERVER_NAME) ? "%h %l %u %t \"%m %U\" %s %b" > : "%h %l %u %t \"%r\" %s %b"; > + String defaultAccessLogPattern = > servername.equalsIgnoreCase(KMS_SERVER_NAME) ? "%h %l %u %t \"%m %U\" %s %b > %D" : "%h %l %u %t \"%r\" %s %b %D"; > ``` You are right. It is better to have perf log disabled. Adding timecost to access logs is a good idea. Do you want to update this patch with this change? - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73856/#review224078 ----------------------------------------------------------- On Feb. 18, 2022, 1:22 p.m., Kirby Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73856/ > ----------------------------------------------------------- > > (Updated Feb. 18, 2022, 1:22 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-3631 > https://issues.apache.org/jira/browse/RANGER-3631 > > > Repository: ranger > > > Description > ------- > > 1. The problematic pattern causes no line break in sql.log . > > > - <pattern>%d [%t] %-5p %C{6} (%F:%L) %msg%n</pattern> > + <pattern>%d [%t] %-5p %C{6} (%F:%L) %msg%n</pattern> > > > 2. "fatal" is a invalid log level in logback > > 3. maxHistory is missing in all. > 4. additivity is missing in "org.apache.ranger.perf" > > > Diffs > ----- > > security-admin/src/main/webapp/WEB-INF/logback.xml > 4b3e7b5717f2ed726fa50018e759dcddab26e19b > > > Diff: https://reviews.apache.org/r/73856/diff/1/ > > > Testing > ------- > > mvn clean build package. > > fresh installation and upgrade > > > Thanks, > > Kirby Zhou > >
