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


Generally I think this is a good plan. This is a core part of the Flume 
functionality and it has taken a fair amount of work to work around all the 
various hadoop kerberos items so I am hopeful that someone who has spent more 
time in this code can review the change.

I have provided my initial feedback below.


flume-ng-core/src/main/java/org/apache/flume/sink/AbstractBucketWriter.java
<https://reviews.apache.org/r/7131/#comment25042>

    This is a hadoop issue and file systems which other sinks such as Rolling 
File sink won't have such an issue. Should this code in a class related to 
hadoop?



flume-ng-core/src/main/java/org/apache/flume/sink/FileBucketWriter.java
<https://reviews.apache.org/r/7131/#comment25040>

    I think we should be using a File object to get the directory here.



flume-ng-core/src/main/java/org/apache/flume/sink/FileBucketWriter.java
<https://reviews.apache.org/r/7131/#comment25041>

    We should do an assertion or log something if mkdirs fails.


- Brock Noland


On Sept. 17, 2012, 3:49 p.m., Ted Malaska wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7131/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2012, 3:49 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> OK, first note this is still a work in progress, but I'm at the point where I 
> need feedback.
> 
> Initial goal was to allow RollingFileSink to do Escape Sequence the same way 
> as HDFSEventSink.
> 
> In order to reuse code, my change attempt to reuse bucketWriter and 
> BucketWriterLinkedHashMap.
> 
> BucketWriter has been broken up into three files.
> 
> > AbstractBucketWriter : Contains all the rollover and escape sequence logic.
> > BucketWriter: Contains the IO logic to HDFS
> > FileBucketWriter: contains the IO logic to normal files
> 
> I also added a couple unit tests to RollingFileSink for new types of roll 
> overs and escape sequences.
> 
> 
> This addresses bug FLUME-1295.
>     https://issues.apache.org/jira/browse/FLUME-1295
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/sink/AbstractBucketWriter.java 
> PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/sink/BucketWriterLinkedHashMap.java
>  PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/sink/FileBucketWriter.java 
> PRE-CREATION 
>   flume-ng-core/src/main/java/org/apache/flume/sink/RollingFileSink.java 
> be640bb 
>   flume-ng-core/src/test/java/org/apache/flume/sink/TestRollingFileSink.java 
> 07fa644 
>   
> flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java
>  6408eb9 
>   
> flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java
>  9a76ecb 
> 
> Diff: https://reviews.apache.org/r/7131/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ted Malaska
> 
>

Reply via email to