You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Neha Narkhede <ne...@gmail.com> on 2014/06/20 23:08:08 UTC
Review Request 22841: Patch for KAFKA-1329
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22841/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1329
https://issues.apache.org/jira/browse/KAFKA-1329
Repository: kafka
Description
-------
Adding topic and consumer metadata refresh capability to the new consumer. Few things to note - 1. The fetch buffer related configs are still a bit awkward until we figure out how to fetch respecting memory management. This patch does not attempt to fix that. 2. This patch just focuses on exercising the new refactored network client to refresh metadata. Metadata currently blocks waiting for a background thread to wake it up. This does not work for the consumer since it is single threaded. I had to change it to accept a -1 wait time which indicates no wait. 3. Added couple unit tests which are still awkward since it is difficult to just test the metadata refresh in the absence of fetch/commit capability
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 522881c972ca42ff4dfb6237a2db15b625334d7e
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 46efc0c8483acacf42b2984ac3f3b9e0a4566187
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java fe93afa24fc20b03830f1d190a276041d15bd3b9
clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 57bc285c20b5af8957bcc5322cd75c021a5af215
clients/src/main/java/org/apache/kafka/common/PartitionInfo.java b15aa2c3ef2d7c4b24618ff42fd4da324237a813
clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 6fe7573973832615976defa37fe0dfbb8f911939
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 044b03061802ee5e8ea4f1995fb0988e1a70e9a7
clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataResponse.java PRE-CREATION
clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 2652c32f123b3bc4b0456d4bc9fbba52c051724c
clients/src/test/java/org/apache/kafka/clients/producer/PartitionerTest.java f06e28ce21e80c1265258ad3ac7900b99e61493d
clients/src/test/java/org/apache/kafka/test/TestUtils.java 76a17e8849bada6bcb025df66a7f20789c0e0300
core/src/test/scala/unit/kafka/consumer/ConsumerTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala 9f04bd38be639cde3e7f402845dbe6ae92e87dc2
core/src/test/scala/unit/kafka/utils/TestUtils.scala 57b2bd5aefc511773a6a384aaac250b5979c0fa4
Diff: https://reviews.apache.org/r/22841/diff/
Testing
-------
Thanks,
Neha Narkhede
Re: Review Request 22841: Patch for KAFKA-1329
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22841/#review46769
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java
<https://reviews.apache.org/r/22841/#comment82344>
In producer this config name does not have the _CONFIG suffix, maybe we should make it consistent with others by adding the suffix in both producer and consumer configs.
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
<https://reviews.apache.org/r/22841/#comment82345>
remove "for best performance".
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
<https://reviews.apache.org/r/22841/#comment82350>
"cluster" is not used. If we just want to set the forceUpdate boolean if necessary we can probably add a new API function in Metadata doing so, instead of using fetch().
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
<https://reviews.apache.org/r/22841/#comment82351>
When the consumer needs to update its consumer metadata, we'd better send the request asap instead of sticking with the lease loaded node. Probably we can just round robin the broker nodes and choose the first one that is ready.
core/src/test/scala/unit/kafka/consumer/ConsumerTest.scala
<https://reviews.apache.org/r/22841/#comment82352>
use ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, etc
- Guozhang Wang
On June 20, 2014, 9:08 p.m., Neha Narkhede wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22841/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 9:08 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1329
> https://issues.apache.org/jira/browse/KAFKA-1329
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Adding topic and consumer metadata refresh capability to the new consumer. Few things to note - 1. The fetch buffer related configs are still a bit awkward until we figure out how to fetch respecting memory management. This patch does not attempt to fix that. 2. This patch just focuses on exercising the new refactored network client to refresh metadata. Metadata currently blocks waiting for a background thread to wake it up. This does not work for the consumer since it is single threaded. I had to change it to accept a -1 wait time which indicates no wait. 3. Added couple unit tests which are still awkward since it is difficult to just test the metadata refresh in the absence of fetch/commit capability
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 522881c972ca42ff4dfb6237a2db15b625334d7e
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 46efc0c8483acacf42b2984ac3f3b9e0a4566187
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java fe93afa24fc20b03830f1d190a276041d15bd3b9
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java 57bc285c20b5af8957bcc5322cd75c021a5af215
> clients/src/main/java/org/apache/kafka/common/PartitionInfo.java b15aa2c3ef2d7c4b24618ff42fd4da324237a813
> clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 6fe7573973832615976defa37fe0dfbb8f911939
> clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 044b03061802ee5e8ea4f1995fb0988e1a70e9a7
> clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 2652c32f123b3bc4b0456d4bc9fbba52c051724c
> clients/src/test/java/org/apache/kafka/clients/producer/PartitionerTest.java f06e28ce21e80c1265258ad3ac7900b99e61493d
> clients/src/test/java/org/apache/kafka/test/TestUtils.java 76a17e8849bada6bcb025df66a7f20789c0e0300
> core/src/test/scala/unit/kafka/consumer/ConsumerTest.scala PRE-CREATION
> core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala 9f04bd38be639cde3e7f402845dbe6ae92e87dc2
> core/src/test/scala/unit/kafka/utils/TestUtils.scala 57b2bd5aefc511773a6a384aaac250b5979c0fa4
>
> Diff: https://reviews.apache.org/r/22841/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Neha Narkhede
>
>