[SPARK-12177] [STREAMING] Update KafkaDStreams to new Kafka 0.9 Consu… #10294
Conversation
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
This comment style is inconsistent with the rest of Spark (also makes it difficult to see what parts of the comment has/hasn't changed).
|
There's a lot of duplications copied from the original code, is it possible to consolidate it? Also if current Kafka client is wire-compatible with Kafka 0.9 brokers, we could still use the current client and wait until most of the uses upgrade the Kafka cluster to 0.9 to change the client. From my thinking maintaining two versions of Kafka in Spark will increase the maintenance burden. |
|
I did this duplicates because I didn't want to broke the current implementation. Do you have some ideas how to do consolidation between this two versions? Maybe we should use something like shim classes for this - set of abstract classes and 2 submodules with old and new implementations? |
nikit-os
force-pushed
the
kafka-09-consumer-api
branch
from
0d1a22b to
328379e
Compare
There was a problem hiding this comment.
probably writing 0L would be better rather than (long) casting
|
Hi @nikit-os I have made a note of some minor observations and suggestions on some of the test cases related to using new consumer. Have a look and let me know if you will need any further info from me on them. Thanks Praveen |
nikit-os
force-pushed
the
kafka-09-consumer-api
branch
from
7e4a843 to
8312792
Compare
|
Hi @praveend ! Thank you for review! I have corrected mistakes. |
|
Is there any feature / perf improvement / etc., in 0.9 that makes this upgrade worth all the change? |
|
Kafka 0.9 introduce new consumer which eliminates the dependence on Zookeeper. It replaces "high-level" and "low-level" consumers and makes working with Kafka much easier. Also new producer and consumer support new security features (such as Kerberos, SSL, unix-like permissions system control). |
|
I'm closing this PR, because I moved my changes to another branch that based on master. Link to new PR - #10681 |
6 participants
…mer API