> On Aug. 31, 2016, 3:27 p.m., Denes Arvay wrote: > > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java, > > lines 538-606 > > <https://reviews.apache.org/r/51541/diff/1/?file=1488879#file1488879line538> > > > > I'm a bit concerned about this copy-paste from the KafkaChannel it > > should be extracted to a utility class. > > If you don't have time for this now please file a jira to > > improve/refactor this later. Thanks. > > Grant Henke wrote: > I agree. I felt the same way initially. The reasons I didn't break it out > into a utility class were: > - It's not clear the correct place it would go > - This code is temporary and should be removed when migration support can > be dropped > - Though very similar there are some differences in the implimentation > > I can open a jira to track consolidation.
I created https://issues.apache.org/jira/browse/FLUME-2985 to track the consolidation. - Grant ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51541/#review147430 ----------------------------------------------------------- On Aug. 30, 2016, 9:47 p.m., Grant Henke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51541/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2016, 9:47 p.m.) > > > Review request for Flume. > > > Bugs: FLUME-2983 > https://issues.apache.org/jira/browse/FLUME-2983 > > > Repository: flume-git > > > Description > ------- > > Similar to FLUME-2972, Offsets tracking the position in Kafka consumers > change from using zookeeper for offset storage to Kafka when moving from > 0.8.x to 0.9.x. > FLUME-2821 makes the client change in the Kafka Source but does not ensure > existing offsets get migrated in order to continue consuming where it left > off. > This patch adds automated logic on startup to check if Kafka offsets exist, > if not and migration is enabled (by default) then copy the offsets from > Zookeeper and commit them to Kafka. > This patch also fixes the backwards incompatibility caused by removing the > zookeeperConnect property. The bootstrap can be looked up if zookeeperConnect > is used. > > > Diffs > ----- > > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannel.java > e7f1f2e > > flume-ng-channels/flume-kafka-channel/src/test/java/org/apache/flume/channel/kafka/TestKafkaChannel.java > e7ae68f > flume-ng-doc/sphinx/FlumeUserGuide.rst 0fd1ec9 > flume-ng-sources/flume-kafka-source/pom.xml 5f5c2a8 > > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java > 86782c3 > > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSourceConstants.java > 1f255f9 > > flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java > a3a2f92 > > flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java > 1598741 > > Diff: https://reviews.apache.org/r/51541/diff/ > > > Testing > ------- > > Unit tests so far. > > > Thanks, > > Grant Henke > >
