----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4445/#review6231 -----------------------------------------------------------
Thanks for the patch Juhani. I was able to run the tests successfully. I have some minor feedback below for your consideration. flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13432> It will be good to cap this penalty amount to a predefined/configured ceiling value. flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java <https://reviews.apache.org/r/4445/#comment13438> There is one slight issue here though - which is if the channel is empty, the sink being attempted to recover will likely return BACKOFF, implying that the sink is normal and has recovered. A minor nit: it will be nice if the process invocation on the failed sink was from within the process() that calls the active Sink. That way the logic stays in one place. - Arvind On 2012-03-22 08:23:00, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4445/ > ----------------------------------------------------------- > > (Updated 2012-03-22 08:23:00) > > > 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 > >
