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 <wa...@gmail.com> on 2015/06/01 02:05:12 UTC

Re: Review Request 34524: Fix KAFKA-2208

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

(Updated June 1, 2015, 12:05 a.m.)


Review request for kafka.


Bugs: KAFKA-2208
    https://issues.apache.org/jira/browse/KAFKA-2208


Repository: kafka


Description (updated)
-------

Incorporate Onur's comments; add logic for removing the whole group from consumer.


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
  clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
  clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
  core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
  core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
  core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
  core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
  core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
  core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 

Diff: https://reviews.apache.org/r/34524/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 34524: Fix KAFKA-2208

Posted by Onur Karaman <ok...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34524/#review86304
-----------------------------------------------------------

Ship it!


I brought up a local vagrant cluster of 2 brokers, 1 zk node, and one worker node. The worker node had a producer and consumer. Both brokers were in the broker-list / bootstrap.servers lists for the producer / consumer, respectively.

I kill -9'd the broker doing the coordination for the consumer. The consumer was able to switch over its coordination to the other broker.

- Onur Karaman


On June 2, 2015, 9:07 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34524/
> -----------------------------------------------------------
> 
> (Updated June 2, 2015, 9:07 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2208
>     https://issues.apache.org/jira/browse/KAFKA-2208
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Incorporate Onur's comments
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
>   core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
>   core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
>   core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
>   core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
>   core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 
> 
> Diff: https://reviews.apache.org/r/34524/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 34524: Fix KAFKA-2208

Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34524/
-----------------------------------------------------------

(Updated June 2, 2015, 9:07 p.m.)


Review request for kafka.


Bugs: KAFKA-2208
    https://issues.apache.org/jira/browse/KAFKA-2208


Repository: kafka


Description
-------

Incorporate Onur's comments


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
  clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
  clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
  core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
  core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
  core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
  core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
  core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
  core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 

Diff: https://reviews.apache.org/r/34524/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 34524: Fix KAFKA-2208

Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34524/
-----------------------------------------------------------

(Updated June 2, 2015, 5:45 p.m.)


Review request for kafka.


Bugs: KAFKA-2208
    https://issues.apache.org/jira/browse/KAFKA-2208


Repository: kafka


Description (updated)
-------

Incorporate Onur's comments


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
  clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
  clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
  clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
  core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
  core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
  core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
  core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
  core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
  core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 

Diff: https://reviews.apache.org/r/34524/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 34524: Fix KAFKA-2208

Posted by Guozhang Wang <wa...@gmail.com>.

> On June 2, 2015, 12:20 a.m., Onur Karaman wrote:
> > It'd rather avoid mixing coordinator failover optimization logic with this rb. Can you undo the changes in ConsumerCoordinator.scala from line 214 down to the bottom of ConsumerCoordinator.scala?

OK agreed. Revert the remove-group logic in maybeRebalance. I kept the function and used it for the other place where we did originally remove the group upon empty member list.


- Guozhang


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


On June 1, 2015, 12:05 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34524/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 12:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2208
>     https://issues.apache.org/jira/browse/KAFKA-2208
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Incorporate Onur's comments; add logic for removing the whole group from consumer.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
>   core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
>   core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
>   core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
>   core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
>   core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 
> 
> Diff: https://reviews.apache.org/r/34524/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 34524: Fix KAFKA-2208

Posted by Onur Karaman <ok...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34524/#review86101
-----------------------------------------------------------


It'd rather avoid mixing coordinator failover optimization logic with this rb. Can you undo the changes in ConsumerCoordinator.scala from line 214 down to the bottom of ConsumerCoordinator.scala?


clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java
<https://reviews.apache.org/r/34524/#comment138003>

    This is missing:
    INCONSISTENT_PARTITION_ASSIGNMENT_STRATEGY (23)



core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala
<https://reviews.apache.org/r/34524/#comment138006>

    Not sure but I think kafka's convention is Errors.foo.code for scala. ReplicaManager and the rest of ConsumerCoordinator does Errors.foo.code, not Errors.foo.code()


- Onur Karaman


On June 1, 2015, 12:05 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34524/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 12:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2208
>     https://issues.apache.org/jira/browse/KAFKA-2208
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Incorporate Onur's comments; add logic for removing the whole group from consumer.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java b2764df11afa7a99fce46d1ff48960d889032d14 
>   clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java ef9dd5238fbc771496029866ece1d85db6d7b7a5 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatResponse.java f548cd0ef70929b35ac887f8fccb7b24c3e2c11a 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 
>   core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala af06ad45cdc46ac3bc27898ebc1a5bd5b1c7b19e 
>   core/src/main/scala/kafka/coordinator/ConsumerGroupMetadata.scala 47bdfa7cc86fd4e841e2b1d6bfd40f1508e643bd 
>   core/src/main/scala/kafka/network/RequestChannel.scala 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b 
>   core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala 5c4cca653b3801df3494003cc40a56ae60a789a6 
>   core/src/test/scala/integration/kafka/api/ConsumerTest.scala a1eed965a148eb19d9a6cefbfce131f58aaffc24 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 
> 
> Diff: https://reviews.apache.org/r/34524/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>