----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39144/#review111685 -----------------------------------------------------------
core/src/main/conf/oozie-log4j.properties (line 90) <https://reviews.apache.org/r/39144/#comment171917> We seem to be going from '.'yyyy-MM-dd to -yyyy-MM-dd. Old files will be ignored due to pattern change. That is fine as it will only be for max days and not frequently accessed by users. But will new pattern be picked up? I don't see any code changes related to that. Can you check Oozie Audit Log tab in UI? XLogUtil.extractInfoForLogWebService() { .... else if (appenderClass.equals("org.apache.log4j.rolling.RollingFileAppender")) { ... log.warn( "Oozie WS " + logType + " log will be disabled, RollingPolicy.FileNamePattern [{0}] should end with " + "'-%d{yyyy-MM-dd-HH}' or '-%d{yyyy-MM-dd-HH}.gz' and also start with the value of " + "log4j.appender." + logType + ".File [{1}]", pattern, logFile); .. } } -%d{yyyy-MM-dd} is not recognized there and it will disable audit logging as far as I see. core/src/main/java/org/apache/oozie/util/OozieRollingPolicy.java (lines 57 - 58) <https://reviews.apache.org/r/39144/#comment171913> Can you call the variables oozieLogDir and logFileName? Makes it easy to understand that the first one is the directory and not the full log file path and second one is just the name. core/src/main/java/org/apache/oozie/util/OozieRollingPolicy.java (line 174) <https://reviews.apache.org/r/39144/#comment171911> This logic is simple and good considering only two formats (day and hour) are supported. Can you create a new jira to cleanup XLogStreamer.getGZFileCreationTime as well? Do mention in the jira that we need to make SimpleDateFormat as a thread local variable in one class to be used by both the methods. core/src/test/java/org/apache/oozie/util/TestOozieRollingPolicy.java (line 62) <https://reviews.apache.org/r/39144/#comment171914> Typo. Should be calendarUnit core/src/test/java/org/apache/oozie/util/TestOozieRollingPolicy.java (line 85) <https://reviews.apache.org/r/39144/#comment171916> calendarUnit == Calendar.HOUR_OF_DAY instead of isAuditLog to determine isHourlyLog. core/src/test/java/org/apache/oozie/util/TestOozieRollingPolicy.java (line 152) <https://reviews.apache.org/r/39144/#comment171915> To make it generic, can you change to boolean isAuditLog- > boolean isHourlyLog if (!isAuditLog) -> if (isHourlyLog) - Rohini Palaniswamy On Oct. 8, 2015, 8:53 p.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39144/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2015, 8:53 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-2312 > https://issues.apache.org/jira/browse/OOZIE-2312 > > > Repository: oozie-git > > > Description > ------- > > oozie doesn't purge audit and error log > > > Diffs > ----- > > core/src/main/conf/oozie-log4j.properties > 2b20ff282477d91549e3f6cdee425674e4cc773b > core/src/main/java/org/apache/oozie/util/OozieRollingPolicy.java > 19ce7f95d3914d006f257e8ad4cc9b68e1d6488c > core/src/test/java/org/apache/oozie/util/TestOozieRollingPolicy.java > 9ae7bec99fa9b47ca05a485c2f47bb3f642a54a2 > > Diff: https://reviews.apache.org/r/39144/diff/ > > > Testing > ------- > > > Thanks, > > Purshotam Shah > >
