You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2013/09/09 23:24:51 UTC
Review Request 14041: KAFKA-1030
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
Review request for kafka.
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs
-----
core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: Patch for KAFKA-1030
Posted by Swapnil Ghike <sg...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26179
-----------------------------------------------------------
Ship it!
Ship It!
- Swapnil Ghike
On Sept. 17, 2013, 6 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2013, 6 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 81bf0bda3229e94ecb6b6aff3ffc9fde852df61b
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: Patch for KAFKA-1030
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26183
-----------------------------------------------------------
Ship it!
Ship It!
- Neha Narkhede
On Sept. 17, 2013, 6 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2013, 6 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 81bf0bda3229e94ecb6b6aff3ffc9fde852df61b
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: Patch for KAFKA-1030
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26180
-----------------------------------------------------------
Ship it!
- Neha Narkhede
On Sept. 17, 2013, 6 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2013, 6 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 81bf0bda3229e94ecb6b6aff3ffc9fde852df61b
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: Patch for KAFKA-1030
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 17, 2013, 6 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1030
Bugs: KAFKA-1030
https://issues.apache.org/jira/browse/KAFKA-1030
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs (updated)
-----
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 81bf0bda3229e94ecb6b6aff3ffc9fde852df61b
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Sept. 10, 2013, 9:46 p.m., Joel Koshy wrote:
> > Following up on the performance concerns that Neha had raised - this will be a significant bottleneck for tools such as the mirror-maker as the rebalance latency will almost certainly multiply.
> >
> > We could consider utilizing the returned children information in ZkRebalancerListener's handleChildChange - i.e., curChilds will be the partitions if the parentPath is a topic. It won't be a trivial change then - we currently recompute the full map of topic/partitions during each rebalance. Now, we will need to "maintain" that data structure. There is also an initial bootstrap problem (that can also be addressed).
> >
> > This makes me wonder if it is better to implement selective rebalance - that way the overhead of going to ZK for partition information (for the affected topics alone) should be acceptable. It will also give us the added benefit of reducing the overall time for rebalance on a topic event.
> >
Hi Joel,
Thanks for the suggestion. I think this is a better fix for this bug, but this would require cache data that is read from ZK it would require longer stabilization and potential-bug-fix time. Since we are getting close to 0.8 final release I think it would be better to implement your proposal later in trunk.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26022
-----------------------------------------------------------
On Sept. 10, 2013, 6:29 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2013, 6:29 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26022
-----------------------------------------------------------
Following up on the performance concerns that Neha had raised - this will be a significant bottleneck for tools such as the mirror-maker as the rebalance latency will almost certainly multiply.
We could consider utilizing the returned children information in ZkRebalancerListener's handleChildChange - i.e., curChilds will be the partitions if the parentPath is a topic. It won't be a trivial change then - we currently recompute the full map of topic/partitions during each rebalance. Now, we will need to "maintain" that data structure. There is also an initial bootstrap problem (that can also be addressed).
This makes me wonder if it is better to implement selective rebalance - that way the overhead of going to ZK for partition information (for the affected topics alone) should be acceptable. It will also give us the added benefit of reducing the overall time for rebalance on a topic event.
- Joel Koshy
On Sept. 10, 2013, 6:29 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2013, 6:29 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 10, 2013, 6:29 p.m.)
Review request for kafka.
Changes
-------
Incorporate Swapnil's comments
Bugs: KAFKA-1030
https://issues.apache.org/jira/browse/KAFKA-1030
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs (updated)
-----
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 10, 2013, 6:28 p.m.)
Review request for kafka.
Bugs: KAFKA-1030
https://issues.apache.org/jira/browse/KAFKA-1030
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs (updated)
-----
core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Sept. 10, 2013, 6:52 a.m., Swapnil Ghike wrote:
> > core/src/main/scala/kafka/client/ClientUtils.scala, lines 97-116
> > <https://reviews.apache.org/r/14041/diff/2/?file=349971#file349971line97>
> >
> > You can reuse ZkUtils.getPartitionAssignmentForTopics instead of writing a new function.
Yeah exactly! Thanks Swapnil.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26012
-----------------------------------------------------------
On Sept. 10, 2013, 6:29 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2013, 6:29 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Swapnil Ghike <sg...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26012
-----------------------------------------------------------
core/src/main/scala/kafka/client/ClientUtils.scala
<https://reviews.apache.org/r/14041/#comment50776>
You can reuse ZkUtils.getPartitionAssignmentForTopics instead of writing a new function.
- Swapnil Ghike
On Sept. 9, 2013, 9:31 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2013, 9:31 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
> On Sept. 9, 2013, 11:44 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/client/ClientUtils.scala, line 100
> > <https://reviews.apache.org/r/14041/diff/2/?file=349971#file349971line100>
> >
> > The concern I have with this change is the potential performance hit to the consumer rebalance process. Since it affects the end-to-end message latency, I would encourage you to run some performance tests for both a MirrorMaker that consumes ~500 topics as well as a consumer that consumes 5 topics, to quantify the performance hit.
> >
> > Also, what do you think about the alternate solution that I described on the JIRA?
Yeah I will do the perf test.
About the alternative approach, that would require the handling api to recognize if the current broker is controller or not; and if yes, hand over the request to the controller module. I think that would complicate the logic. What do you think?
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26005
-----------------------------------------------------------
On Sept. 9, 2013, 9:31 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2013, 9:31 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/#review26005
-----------------------------------------------------------
core/src/main/scala/kafka/client/ClientUtils.scala
<https://reviews.apache.org/r/14041/#comment50755>
The concern I have with this change is the potential performance hit to the consumer rebalance process. Since it affects the end-to-end message latency, I would encourage you to run some performance tests for both a MirrorMaker that consumes ~500 topics as well as a consumer that consumes 5 topics, to quantify the performance hit.
Also, what do you think about the alternate solution that I described on the JIRA?
- Neha Narkhede
On Sept. 9, 2013, 9:31 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14041/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2013, 9:31 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1030
> https://issues.apache.org/jira/browse/KAFKA-1030
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Using the approach of reading directly from ZK.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
>
> Diff: https://reviews.apache.org/r/14041/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 14041: MetadataResponse during Consumer's Rebalance
Process maybe Stale
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 9, 2013, 9:31 p.m.)
Review request for kafka.
Summary (updated)
-----------------
MetadataResponse during Consumer's Rebalance Process maybe Stale
Bugs: KAFKA-1030
https://issues.apache.org/jira/browse/KAFKA-1030
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs
-----
core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: KAFKA-1030
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 9, 2013, 9:30 p.m.)
Review request for kafka.
Bugs: KAFKA-1030
https://issues.apache.org/jira/browse/KAFKA-1030
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs
-----
core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 14041: KAFKA-1030
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14041/
-----------------------------------------------------------
(Updated Sept. 9, 2013, 9:30 p.m.)
Review request for kafka.
Repository: kafka
Description
-------
Using the approach of reading directly from ZK.
Diffs (updated)
-----
core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a1d23ca5a9ecf86e3cb34be418b9c0c943
Diff: https://reviews.apache.org/r/14041/diff/
Testing
-------
unit tests
Thanks,
Guozhang Wang