----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10944/#review20956 -----------------------------------------------------------
Hari, this is great! I have a few questions/comments below. I just did an initial pass. Also since we are making so many classes public should we add visibility annotations to those classes? flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java <https://reviews.apache.org/r/10944/#comment43254> Curious as to what byte this ends up representing? flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java <https://reviews.apache.org/r/10944/#comment43255> this. is not consistent with the above flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java <https://reviews.apache.org/r/10944/#comment43256> Let's log the offset and file like the message above. flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java <https://reviews.apache.org/r/10944/#comment43250> Curious why we have "" + ".old"? flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java <https://reviews.apache.org/r/10944/#comment43253> This should be typed IllegalStateException no? flume-tools/src/main/java/org/apache/flume/tools/FileChannelIntegrityTool.java <https://reviews.apache.org/r/10944/#comment43262> Curious as to what case would make eventPosition == -1? - Brock Noland On May 23, 2013, 8:54 p.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10944/ > ----------------------------------------------------------- > > (Updated May 23, 2013, 8:54 p.m.) > > > Review request for Flume. > > > Description > ------- > > Patch to add a checksum to events and replace them with a noop event using a > tool, if corrupt. > > > This addresses bug FLUME-1586. > https://issues.apache.org/jira/browse/FLUME-1586 > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/CorruptEventException.java > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > cc0d38a > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java > 1918baa > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java > d3db896 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java > d9a2a9b > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/NoopRecordException.java > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java > 4235a79 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/ReplayHandler.java > fc47b23 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java > d6897e1 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java > 073042f > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/proto/ProtosFactory.java > 4860ac2 > flume-ng-channels/flume-file-channel/src/main/proto/filechannel.proto > 1e668d2 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java > 0f7d14d > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java > 54978f8 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java > bef22ef > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestTransactionEventRecordV3.java > f403422 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java > 563dbcc > flume-ng-dist/pom.xml 1a44626 > flume-ng-dist/src/main/assembly/bin.xml b6a8c8a > flume-ng-dist/src/main/assembly/src.xml 7fafab8 > flume-tools/pom.xml PRE-CREATION > > flume-tools/src/main/java/org/apache/flume/tools/FileChannelIntegrityTool.java > PRE-CREATION > flume-tools/src/main/java/org/apache/flume/tools/FlumeTool.java > PRE-CREATION > flume-tools/src/main/java/org/apache/flume/tools/FlumeToolType.java > PRE-CREATION > flume-tools/src/main/java/org/apache/flume/tools/FlumeToolsMain.java > PRE-CREATION > > flume-tools/src/test/java/org/apache/flume/tools/TestFileChannelIntegrityTool.java > PRE-CREATION > flume-tools/src/test/java/org/apache/flume/tools/TestFlumeToolsMain.java > PRE-CREATION > pom.xml a6992f6 > > Diff: https://reviews.apache.org/r/10944/diff/ > > > Testing > ------- > > Added unit tests when corrupt and noop events are encountered. I will add > tests for the tool as well soon. I have not yet tested the tool completely. > This patch aims at gathering feedback on the approach. > > > Thanks, > > Hari Shreedharan > >
