> On Oct. 5, 2016, 9:12 p.m., Jeff Holoman wrote: > > Can you elaborate on "Testing passed". I think it would make sense to > > unit-test this in order to ensure the counters are incrementing properly.
As this is a trivial change and we have already working counters like TIMER_KAFKA_EVENT_GET, COUNTER_EVENT_PUT_SUCCESS and COUNTER_EVENT_TAKE_SUCCESS in place so thought this would work in the same way. > On Oct. 5, 2016, 9:12 p.m., Jeff Holoman wrote: > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannel.java, > > line 533 > > <https://reviews.apache.org/r/52550/diff/1/?file=1522919#file1522919line533> > > > > I don't think this is in the correct place. The previous line adds the > > entire batch to the Put Success count. I think we want to add this in the > > doPut method instead? Yes, you're correct. I was thinking to also increment it even when trying to commit the get/put. But that is actually not a get/put attempt so I will revert it. > On Oct. 5, 2016, 9:12 p.m., Jeff Holoman wrote: > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannel.java, > > line 550 > > <https://reviews.apache.org/r/52550/diff/1/?file=1522919#file1522919line550> > > > > Same thing here. I don't see why we need this here if we are already > > incrementing in the doTake()? Fixed as above comment - Umesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52550/#review151557 ----------------------------------------------------------- On Oct. 5, 2016, 10:51 a.m., Umesh Chaudhary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52550/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2016, 10:51 a.m.) > > > Review request for Flume. > > > Repository: flume-git > > > Description > ------- > > Added appropriate function calls to capture eventTakeAttemptCount and > eventPutAttemptCount in KafkaChannel > > > Diffs > ----- > > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannel.java > 66b553a > > Diff: https://reviews.apache.org/r/52550/diff/ > > > Testing > ------- > > Testing Passed. > > > Thanks, > > Umesh Chaudhary > >
