----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15779/#review29389 -----------------------------------------------------------
Juhani, I don't mind the idea of this patch, but I am not sure this works. If the flush fails, we ignore the failure and we set isOpen to false, we are not going to try to close the file again (barring retries which is possible in newer versions of hadoop and flume). If we don't try to close or flush again, the data could still be stuck in the local buffers and we risk data loss. The real issue is that there is some underlying failure in HDFS, which should be handled. I think the real solution is to retry closes, which we added in Flume a few months back, provided that version of HDFS supports it. Does that make sense? - Hari Shreedharan On Nov. 22, 2013, 8:38 a.m., Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15779/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2013, 8:38 a.m.) > > > Review request for Flume. > > > Repository: flume-git > > > Description > ------- > > https://issues.apache.org/jira/browse/FLUME-2245 > > Originally the flush() seemed superfluous however without it one of the unit > tests breaks. > > By moving on beyond regardless of the flush succeeding or not we allow the > backing stream to actually get closed and reopened. While the real problem is > with the HDFS stream not recovering this workaround seems necessary as > otherwise appends will continue to fail until a restart. > > Similarly HDFSDataStream and HDFSCompressedDataStream are closed regardless > of the success of serialization/flushing. The exception should be propagated > and cause a rollback so no data loss occurs. > > > Diffs > ----- > > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java > 200d457 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSCompressedDataStream.java > 5518547 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSDataStream.java > e20d1ee > > Diff: https://reviews.apache.org/r/15779/diff/ > > > Testing > ------- > > Existing unit tests pass. > > I'm still trying to figure out a way to recreate the issue as it is hard to > determine the exact cause > > > Thanks, > > Juhani Connolly > >
