> On 2012-03-22 17:07:01, Arvind Prabhakar wrote: > > Thanks for the patch Juhani. I was able to run the tests successfully. I > > have some minor feedback below for your consideration.
thanks for running the tests. back to normal on my end too > On 2012-03-22 17:07:01, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java, > > line 90 > > <https://reviews.apache.org/r/4445/diff/1/?file=94575#file94575line90> > > > > It will be good to cap this penalty amount to a predefined/configured > > ceiling value. Added a config variable > On 2012-03-22 17:07:01, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/sink/FailoverSinkProcessor.java, > > lines 120-121 > > <https://reviews.apache.org/r/4445/diff/1/?file=94575#file94575line120> > > > > 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. I got rid of the queue subclass and put the code in process... Though I'm not sure if that is the easiest way for the human brain to parse it... I also changed things so that a backoff results in being returned to the failed list without a penalty - Juhani ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4445/#review6231 ----------------------------------------------------------- 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 > >
