-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8596/#review14708
-----------------------------------------------------------


Hi Mike!

Great patch!  There are a couple issues below but otherwise I think it's ready 
for a commit!


flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java
<https://reviews.apache.org/r/8596/#comment31192>

    It's quite slow with such a large batch size. 4 minutes to transfer 60MB of 
data to a local agent (memory channel and null sink). It would be nice if that 
was configurable. That could be a follow up JIRA but it'd be nice to set this 
be a command line option.



flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java
<https://reviews.apache.org/r/8596/#comment31190>

    I know you didn't change this, but I hit this while testing. That exception 
should be logged in the logger.error().



flume-ng-core/src/main/java/org/apache/flume/client/avro/ReliableSpoolingFileEventReader.java
<https://reviews.apache.org/r/8596/#comment31189>

    It's possible for currentFile to be absent.



flume-ng-core/src/main/java/org/apache/flume/serialization/LineDeserializer.java
<https://reviews.apache.org/r/8596/#comment31191>

    Seeing the error below. Guessing it's cause the last file during a run will 
be closed twice. One in retireCurrentFile and once in the close method. This 
was doubly ugly for me because it hid an exception being thrown in 
retireCurrentFile due to a file name violation.
    
    
    java.nio.channels.ClosedChannelException
        at sun.nio.ch.FileChannelImpl.ensureOpen(FileChannelImpl.java:88)
        at sun.nio.ch.FileChannelImpl.position(FileChannelImpl.java:265)
        at 
org.apache.flume.serialization.ResettableFileInputStream.seek(ResettableFileInputStream.java:212)
        at 
org.apache.flume.serialization.ResettableFileInputStream.reset(ResettableFileInputStream.java:204)
        at 
org.apache.flume.serialization.LineDeserializer.reset(LineDeserializer.java:102)
        at 
org.apache.flume.serialization.LineDeserializer.close(LineDeserializer.java:107)
        at 
org.apache.flume.client.avro.ReliableSpoolingFileEventReader.close(ReliableSpoolingFileEventReader.java:224)
        at 
org.apache.flume.client.avro.AvroCLIClient.run(AvroCLIClient.java:217)
        at 
org.apache.flume.client.avro.AvroCLIClient.main(AvroCLIClient.java:71)
    
    


- Brock Noland


On Dec. 18, 2012, 11:21 p.m., Mike Percy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8596/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2012, 11:21 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Defines EventDeserializer interface and uses it from the spooling source. 
> Progress is persisted as bytes are read from the underlying file.
> 
> 
> This addresses bug FLUME-1632.
>     https://issues.apache.org/jira/browse/FLUME-1632
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/pom.xml 0224519 
>   flume-ng-core/src/main/avro/TransferStateFileMeta.avsc PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java 
> 37e9ffa 
>   
> flume-ng-core/src/main/java/org/apache/flume/client/avro/BufferedLineReader.java
>  718e1b2 
>   flume-ng-core/src/main/java/org/apache/flume/client/avro/EventReader.java 
> PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/client/avro/LineReader.java 
> 904f22c 
>   
> flume-ng-core/src/main/java/org/apache/flume/client/avro/ReliableEventReader.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/client/avro/ReliableSpoolingFileEventReader.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/client/avro/SimpleTextLineEventReader.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/client/avro/SpoolingFileLineReader.java
>  8362299 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/DurablePositionTracker.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventDeserializer.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventDeserializerFactory.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventDeserializerType.java
>  PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerDe.java 
> PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializer.java
>  a418935 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializerFactory.java
>  75853a9 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/EventSerializerType.java
>  afe8ed8 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/LineDeserializer.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/PositionTracker.java
>  PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/serialization/Resettable.java 
> PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/ResettableFileInputStream.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/serialization/ResettableInputStream.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java 
> 61824d8 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySourceConfigurationConstants.java
>  806a661 
>   flume-ng-core/src/main/java/org/apache/flume/tools/PlatformDetect.java 
> PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/client/avro/TestBufferedLineReader.java
>  169abe5 
>   
> flume-ng-core/src/test/java/org/apache/flume/client/avro/TestSpoolingFileLineReader.java
>  740bc98 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/ResettableTestStringInputStream.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestDurablePositionTracker.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestLineDeserializer.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestReliableSpoolingFileEventReader.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestResettableFileInputStream.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java
>  6e87b21 
>   flume-ng-core/src/test/resources/TestResettableFileInputStream_1.avro 
> PRE-CREATION 
>   
> flume-ng-core/src/test/resources/TestResettableFileInputStream_1.truncated.avro
>  PRE-CREATION 
>   pom.xml b934c1d 
> 
> Diff: https://reviews.apache.org/r/8596/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Mike Percy
> 
>

Reply via email to