----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48161/#review137566 -----------------------------------------------------------
flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirMatcher.java (line 98) <https://reviews.apache.org/r/48161/#comment202720> actually this should be "consists of" but I'll just fix it on commit if I remember to do it flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirMatcher.java (line 179) <https://reviews.apache.org/r/48161/#comment202725> Why negate? Why not write this part as: currentParentDirMTime >= lastCheckedTime? Also I am confused about why you are rounding this down to the nearest second. Would you mind explaining the reasoning behind your logic? I don't understand what the intention is, but I don't think it's working as expected. Below is an example of why I think it's not working. Example: Iteration 1) System.currentTimeMillis() returns 1465931328009 so we round this down to 1465931328000 and assign that to 'now'. currentParentDirMTime = 1465931328005 which is before the after 'now' so we process the directory. We also set lastSeenParentDirMTime = 1465931328005 We assign lastCheckedTime = 1465931328000 Iteration 2) System.currentTimeMillis() returns 1465931328500 so we round this down to 1465931328000 and assign that to now (turns out this is the same as before). currentParentDirMTime is still 1465931328005 because it hasn't changed since iteration 1. lastCheckedTime is 1465931328000 which is less than currentParentDirMTime so we process the directory again. Expected result: We don't process the same directory again unnecessarily. flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirMatcher.java (line 225) <https://reviews.apache.org/r/48161/#comment202727> Make this return void and specify that 'files' is sorted in-place flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirMatcher.java (line 227) <https://reviews.apache.org/r/48161/#comment202726> style nit: Maintain spacing according to the Java code conventions: http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#454 This should read like: for (File f : files) { - Mike Percy On June 14, 2016, 4:03 p.m., Attila Simon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48161/ > ----------------------------------------------------------- > > (Updated June 14, 2016, 4:03 p.m.) > > > Review request for Flume. > > > Bugs: FLUME-2918 > https://issues.apache.org/jira/browse/FLUME-2918 > > > Repository: flume-git > > > Description > ------- > > The way TailDir source checks which files should be tracked was improved. > Existing implementation caused unneccessary high CPU usage for huge (+50K > files) directories. This fix allows users to eliminate continous listing of > parent directory (on each Source.process invocation) and introduce a more > performant method for listing&matching files. > > used java.nio.file.DirectoryStream to filter files > made pattern match calculation optionally cached > added junit tests > added javadoc > added license > > > Diffs > ----- > > > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java > 5b6d465 > > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TailFile.java > eabd357 > > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirMatcher.java > PRE-CREATION > > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java > 8816327 > > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java > 6165276 > > flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirMatcher.java > PRE-CREATION > > flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java > f9e614c > > Diff: https://reviews.apache.org/r/48161/diff/ > > > Testing > ------- > > mvn clean install -DskipTests -> built > junit tests for flume-taildir-source module -> passed > > > Thanks, > > Attila Simon > >
