flume-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Percy <mpe...@apache.org>
Subject Re: Review Request 51541: FLUME-2983: Handle offset migration in the new Kafka Source
Date Thu, 01 Sep 2016 00:32:03 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51541/#review147505
-----------------------------------------------------------



not very happy about the copy/paste but this is an important fix. we should clean it up later
though. just a few comments


flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1264)
<https://reviews.apache.org/r/51541/#comment214684>

    mind wrapping these extremely long lines more similarly to the existing text? here and
on the next line



flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1265)
<https://reviews.apache.org/r/51541/#comment214695>

    How about: "If no Zookeeper offset is found, the Kafka configuration kafka.consumer.auto.offset.reset
defines how offsets are handled."



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java
(line 368)
<https://reviews.apache.org/r/51541/#comment214688>

    nit: indent by 4 spaces here



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java
(line 449)
<https://reviews.apache.org/r/51541/#comment214689>

    Could use a little doc comment or something



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java
(line 39)
<https://reviews.apache.org/r/51541/#comment214694>

    Why this change?



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java
(line 83)
<https://reviews.apache.org/r/51541/#comment214693>

    I liked the old method name better... j/k


- Mike Percy


On Aug. 31, 2016, 8:46 a.m., Grant Henke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51541/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2016, 8:46 a.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message