----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63027/#review188283 -----------------------------------------------------------
ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java Lines 85 (patched) <https://reviews.apache.org/r/63027/#comment265275> This variable is not used for anything, so let's get rid of it, and also the function that calculates it in FileUtils ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java Lines 396 (patched) <https://reviews.apache.org/r/63027/#comment265279> No need to expose threadGroup ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java Lines 437 (patched) <https://reviews.apache.org/r/63027/#comment265278> No need to expose multiFolder, and the class may access the variable directly Also no need to expose logFileDetacherThread, and logFilePathUpdaterThread ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/InputFile.java Lines 37 (patched) <https://reviews.apache.org/r/63027/#comment265274> This should go to init(), it doesn't have to be done over and over again till the input is ready ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/monitor/AbstractLogFileMonitor.java Lines 31 (patched) <https://reviews.apache.org/r/63027/#comment265277> inputFile and deteachTime could be protected, thus no getter is needed for the extending classes to access them ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/monitor/AbstractLogFileMonitor.java Lines 45 (patched) <https://reviews.apache.org/r/63027/#comment265276> waitInterval should not be exposed at all ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/util/FileUtil.java Lines 98 (patched) <https://reviews.apache.org/r/63027/#comment265273> This is a utility class performing a general operation, do not link it's name to it's purpose, so let's call this something like getFilesByPathPattern ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/util/FileUtil.java Lines 105 (patched) <https://reviews.apache.org/r/63027/#comment265272> There is a potential hazard, that the folderBeforeRegex repeated after the first *, for example: /var/log/*/var/log/... not likely, but for sure instead of replacing the folderBeforeRegex use substring() - Miklos Gergely On Oct. 16, 2017, 1:03 p.m., Oliver Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63027/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2017, 1:03 p.m.) > > > Review request for Ambari, Miklos Gergely and Robert Nettleton. > > > Bugs: AMBARI-21145 > https://issues.apache.org/jira/browse/AMBARI-21145 > > > Repository: ambari > > > Description > ------- > > Allow wildcards for handling log directories. Using ant library for this. (on > trunk, the solution will be JDK 8 based, so its a temporal hack for 2.6.1, > for trunk some classes have cleaned up, so the implementation there will be > different) > > The idea is if we have a folder pattern, we will craete a thread group, and > there will be different threads for every matching input (now only for tail > file) > > Also there is 2 new thread introduced: > - one which checks is the monitored file too old, if it is, stop the thread > for that (detach) > - an another one which checks there is any new file available (like log files > with date patterns in its name), and if it is, update the monitoring thread > with that (log path upgrade) > > Introduced some new configuration as well for inputs: > - path_update_interval_min: the period in minutes for checking new files > (default: 5, based on detach values, its possible that a new input wont be > monitored) > - detach_interval_min: the period in minutes for checking which files are too > old (default: 300) > - detach_time_min: the period in minutes when we flag a file is too old > (default: 2000) > > Also fix an issue which cause that we cannot use grok field names more deeply > (use deep_extract param for this one in grok filter) and i updated the docker > container as well to make it work with branch-2.6 (and use different version > for container in order to keep that) > > > Diffs > ----- > > ambari-logsearch/ambari-logsearch-logfeeder/pom.xml 49122e8 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java > 7e2da70 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java > 9f54d8a > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/InputFile.java > 3737839 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/monitor/AbstractLogFileMonitor.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/monitor/LogFileDetachMonitor.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/monitor/LogFilePathUpdateMonitor.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/logconfig/LogConfigHandler.java > 0ece637 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/util/FileUtil.java > ffd6cec > ambari-logsearch/docker/Dockerfile dfa1462 > ambari-logsearch/docker/logsearch-docker.sh a2df90f > > > Diff: https://reviews.apache.org/r/63027/diff/1/ > > > Testing > ------- > > done, no UTs here, FTs are in progress > > > Thanks, > > Oliver Szabo > >
