----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4445/#review6265 -----------------------------------------------------------
Thanks for making the changes Juhani. Some feedback follows. flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13571> The max penalty calculated during configure of the sink processor should be applied here to enforce the ceiling. flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13570> This seems like a typo. Perhaps you want to do something like maxPenalty = context.getInteger(CONF_KEY_MAX_PENALTY, DEFAULT_MAX_PENALTY); flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13568> It will be good to log this exception so we have a trace of the failures that are happening. flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13569> Log the exception. Rest of the changes look good to me. - Arvind On 2012-03-23 02:41:03, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4445/ > ----------------------------------------------------------- > > (Updated 2012-03-23 02:41:03) > > > Review request for Flume. > > > Summary > ------- > > As discussed in the JIRA item, I modified FailoverSink to deal with all > exceptions. > Now a sink that fails will be put onto a failed links queue, from which a > recovery will be attempted after a timeout. Each sequential failure the > timeout will increase. I am open to other methods of increasing the > timeout(maybe add on a ceiling?) > > > This addresses bug FLUME-1030. > https://issues.apache.org/jira/browse/FLUME-1030 > > > Diffs > ----- > > > flume-ng-core/src/test/java/org/apache/flume/sink/TestFailoverSinkProcessor.java > 195c121 > > flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java > 7eada57 > > Diff: https://reviews.apache.org/r/4445/diff > > > Testing > ------- > > Modified the test for the new functionality, new test passes > > No other tests should be affected, but my environment was having some weird > problems. I'll look into them tomorrow, just leaving this up so people can > have a browse and will confirm tests passing tomorrow > > > Thanks, > > Juhani > >
