Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

> On Jan. 23, 2015, 4:47 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 259
> > 
> >
> > => If set along with --delete
> > 
> > I'm not sure if I fully understood the purpose of force-delete. 
> > Basically, the only time safe for deleting a consumer group's offset 
> > information is if there are no live consumers in that group anymore. 
> > 
> > If so, --force-delete would mean deleting even if that is not true. 
> > This is pretty disruptive and I can't think of any case where this action 
> > will be useful. 
> > 
> > Thoughts?
> 
> Onur Karaman wrote:
> There are currently four types of delete:
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete 
> --group g1 --group g5
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete 
> --group g3 --group g4 --topic t2
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete 
> --topic t1
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete 
> --topic t3 --force-delete
> 
> --force-delete only applies to topic-wide delete. My concern was that 
> when you do a topic-wide delete, you can potentially impact many consumer 
> groups. So by default, topic-wide delete first checks if the topic still 
> exists. My reasoning was that if a topic still exists during a topic-wide 
> offset delete, it probably wasn't intentional. It overrides the default 
> behavior by ignoring the topic-existance check.
> 
> One complication of the topic existance check is the following scenario:
> 1. We delete topic t.
> 2. t gets recreated due to some producers still producing events to t.
> 3. We try to do a topic-wide offset delete on t.
> 4. The check will prevent the offset delete from happening.
> 
> --force-delete attempts to address that scenario.
> 
> I agree that it is not safe to delete offsets while the group is active, 
> and none of the 4 deletes currently check for this. While it is documented, 
> it makes more sense to push this into the code.

Right, there are really 2 problems being discussed here -
1. Definition of a safe delete in any of the 4 options 
2. Justification for providing the --force-delete option

For 1., it is easier if we clearly state when any of these delete operations is 
safe, model that through the code. I think it is only when there are no active 
consumers in the group.

For 2., I'm not sure that option is very useful and I'm wondering if we can do 
without it. 

Also, let's simplify topic-wide delete to also mean the same - consumer 
information will be deleted if there are no active consumers in the group.


- Neha


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


On Jan. 22, 2015, 10:32 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 22, 2015, 10:32 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 30, 2015, 7:10 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
54755e8dd3f23ced313067566cd4ea867f8a496e 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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


Onur, we should be able to check in after these review comments are addressed. 
Also, how would deleting offsets for a group work when the offset storage is 
Kafka? It's fine to not address it in this patch. Can you please create a JIRA 
to handle that?


core/src/main/scala/kafka/admin/AdminUtils.scala


This is confusing. I think we decided we will keep the definition of a 
valid delete consistent. In this case, we should just detect the inactive 
consumer groups for this topic and delete those. This check can be removed, I 
think.



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


lines too long



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


same here



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


Suggest you create a new local variable for the option docs. That way the 
lines won't be so long. Try to stick to within a width of 100.



core/src/main/scala/kafka/utils/ZkUtils.scala


Let's add consumerGroupOwners inside ZKGroupDirs so we don't have to 
hardcode zk paths like this.



core/src/main/scala/kafka/utils/ZkUtils.scala


same here



core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala


this test probably should have multiple topics and ensure that the API 
deletes only the topic provided?



core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala


Maybe include a topic unrelated to both consumer groups here as well?



core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala


I think these test names are very long. It is definitely useful to have 
test names that convey the purpose but since all tests here include deleting 
consumer groups, I suggest you take that out of all the test names. So 
"DeleteConsumerGroupInfo" can be dropped.



core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala


This test is odd. As I suggested earlier, valid delete should only have one 
rule - active consumer groups will not be deleted. It is alright to delete 
consumer group information for a topic if the topic exists.



core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala


verifyTopicDeletion is duplicated across 2 tests now. It is useful to 
refactor it into maybe TestUtils?


- Neha Narkhede


On Jan. 30, 2015, 7:10 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 30, 2015, 7:10 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Feb. 2, 2015, 5:42 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/utils/ZkUtils.scala, line 758
> > 
> >
> > Let's add consumerGroupOwners inside ZKGroupDirs so we don't have to 
> > hardcode zk paths like this.

I agree that it's better to avoid hardcoding zk paths. There's a bit of a 
naming issue in that ZKGroupTopicDirs will now have methods like:
consumerOffsetDir
consumerGroupOffsetDir // inherited from ZKGroupDirs
consumerOwnerDir
consumerGroupOwnerDir // inherited from ZKGroupDirs


- Onur


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


On Jan. 30, 2015, 7:10 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 30, 2015, 7:10 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Feb. 4, 2015, 11:41 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
54755e8dd3f23ced313067566cd4ea867f8a496e 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Feb. 2, 2015, 5:42 p.m., Neha Narkhede wrote:
> > Onur, we should be able to check in after these review comments are 
> > addressed. Also, how would deleting offsets for a group work when the 
> > offset storage is Kafka? It's fine to not address it in this patch. Can you 
> > please create a JIRA to handle that?

Clark brought up some suggestions on the tool:
- describe currently just shows the log end offset. It might also be useful to 
show the log start offset.
- describe should ideally compare results (current offset, lag, and maybe 
ownership) using both kafka and zookeeper side-by-side.

If we were to follow through with these suggestions, should they be applied in 
a later patch?

As for kafka offset storage: I think that's out of the scope of this patch. I 
made a separate jira for that: https://issues.apache.org/jira/browse/KAFKA-1921


- Onur


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


On Feb. 4, 2015, 11:41 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 4, 2015, 11:41 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 23, 2015, 4:47 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 114
> > 
> >
> > How is this tool going to behave if the consumer's offset information 
> > is stored in kafka, not zookeeper?
> > 
> > The assumption of the user would be to handle that case transparently 
> > as well.

Addressed this in a later comment. Adding the JIRA to this for completeness: 
https://issues.apache.org/jira/browse/KAFKA-1921


- Onur


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


On Feb. 4, 2015, 11:41 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 4, 2015, 11:41 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Feb. 5, 2015, 2:03 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
54755e8dd3f23ced313067566cd4ea867f8a496e 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Feb. 5, 2015, 11:01 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
54755e8dd3f23ced313067566cd4ea867f8a496e 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Feb. 2, 2015, 5:42 p.m., Neha Narkhede wrote:
> > Onur, we should be able to check in after these review comments are 
> > addressed. Also, how would deleting offsets for a group work when the 
> > offset storage is Kafka? It's fine to not address it in this patch. Can you 
> > please create a JIRA to handle that?
> 
> Onur Karaman wrote:
> Clark brought up some suggestions on the tool:
> - describe currently just shows the log end offset. It might also be 
> useful to show the log start offset.
> - describe should ideally compare results (current offset, lag, and maybe 
> ownership) using both kafka and zookeeper side-by-side.
> 
> If we were to follow through with these suggestions, should they be 
> applied in a later patch?
> 
> As for kafka offset storage: I think that's out of the scope of this 
> patch. I made a separate jira for that: 
> https://issues.apache.org/jira/browse/KAFKA-1921

I attached another sample run.

There's a known issue regarding deleted topics and describing groups. Let's say 
group g consumes from topic t. If we delete topic t using kafka-topics.sh and 
then try to describe group g using kafka-consumer-groups.sh, it won't show any 
of the partition rows for topic t even though the group has offset information 
for t still in zk.


- Onur


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


On Feb. 5, 2015, 11:01 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 5, 2015, 11:01 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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

Ship it!


Thanks for attaching the latest run of the tool. I observed that we should get 
rid of zookeeper error message from 
Could not fetch offset from zookeeper for group g1 partition [t1,0] due to 
org.apache.zookeeper.KeeperException$NoNodeException: KeeperErrorCode = NoNode 
for /consumers/g1/offsets/t1/0.

It is somewhat unreadable and not very useful for the user. The best way to 
handle this in a user facing tool is to explain the cause, rather than use the 
error message directly.

Other than that, the patch looks good. Once you fix the above problem, I will 
check it in.

- Neha Narkhede


On Feb. 5, 2015, 11:01 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 5, 2015, 11:01 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Feb. 9, 2015, 10:37 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
54755e8dd3f23ced313067566cd4ea867f8a496e 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Feb. 7, 2015, 3:33 p.m., Neha Narkhede wrote:
> > Thanks for attaching the latest run of the tool. I observed that we should 
> > get rid of zookeeper error message from 
> > Could not fetch offset from zookeeper for group g1 partition [t1,0] due to 
> > org.apache.zookeeper.KeeperException$NoNodeException: KeeperErrorCode = 
> > NoNode for /consumers/g1/offsets/t1/0.
> > 
> > It is somewhat unreadable and not very useful for the user. The best way to 
> > handle this in a user facing tool is to explain the cause, rather than use 
> > the error message directly.
> > 
> > Other than that, the patch looks good. Once you fix the above problem, I 
> > will check it in.

Updated the rb with a hopefully more readable output and attached another 
sample run.

I take it that the known issue regarding deleted topics and describing groups I 
mentioned earlier should get addressed in a later patch?


- Onur


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


On Feb. 9, 2015, 10:37 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 9, 2015, 10:37 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

> On Feb. 2, 2015, 5:42 p.m., Neha Narkhede wrote:
> > Onur, we should be able to check in after these review comments are 
> > addressed. Also, how would deleting offsets for a group work when the 
> > offset storage is Kafka? It's fine to not address it in this patch. Can you 
> > please create a JIRA to handle that?
> 
> Onur Karaman wrote:
> Clark brought up some suggestions on the tool:
> - describe currently just shows the log end offset. It might also be 
> useful to show the log start offset.
> - describe should ideally compare results (current offset, lag, and maybe 
> ownership) using both kafka and zookeeper side-by-side.
> 
> If we were to follow through with these suggestions, should they be 
> applied in a later patch?
> 
> As for kafka offset storage: I think that's out of the scope of this 
> patch. I made a separate jira for that: 
> https://issues.apache.org/jira/browse/KAFKA-1921
> 
> Onur Karaman wrote:
> I attached another sample run.
> 
> There's a known issue regarding deleted topics and describing groups. 
> Let's say group g consumes from topic t. If we delete topic t using 
> kafka-topics.sh and then try to describe group g using 
> kafka-consumer-groups.sh, it won't show any of the partition rows for topic t 
> even though the group has offset information for t still in zk.

Why don't we fix this issue?


- Neha


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


On Feb. 9, 2015, 10:37 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 9, 2015, 10:37 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Feb. 2, 2015, 5:42 p.m., Neha Narkhede wrote:
> > Onur, we should be able to check in after these review comments are 
> > addressed. Also, how would deleting offsets for a group work when the 
> > offset storage is Kafka? It's fine to not address it in this patch. Can you 
> > please create a JIRA to handle that?
> 
> Onur Karaman wrote:
> Clark brought up some suggestions on the tool:
> - describe currently just shows the log end offset. It might also be 
> useful to show the log start offset.
> - describe should ideally compare results (current offset, lag, and maybe 
> ownership) using both kafka and zookeeper side-by-side.
> 
> If we were to follow through with these suggestions, should they be 
> applied in a later patch?
> 
> As for kafka offset storage: I think that's out of the scope of this 
> patch. I made a separate jira for that: 
> https://issues.apache.org/jira/browse/KAFKA-1921
> 
> Onur Karaman wrote:
> I attached another sample run.
> 
> There's a known issue regarding deleted topics and describing groups. 
> Let's say group g consumes from topic t. If we delete topic t using 
> kafka-topics.sh and then try to describe group g using 
> kafka-consumer-groups.sh, it won't show any of the partition rows for topic t 
> even though the group has offset information for t still in zk.
> 
> Neha Narkhede wrote:
> Why don't we fix this issue?

I think this should be addressed in a separate JIRA because this patch has been 
around for a while and keeps getting bigger. The following ticket contains a 
sample run showing the bug:
https://issues.apache.org/jira/browse/KAFKA-1942


- Onur


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


On Feb. 9, 2015, 10:37 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 9, 2015, 10:37 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Eric Olander]

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



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


This can be made a little simpler -

getConsumer(zkClient, brokerId).foreach{ consumer =>
  lines 218-225 kept in here
}

then the case on 226 is no longer needed and the temp variable consumerOpt 
is eliminated.


- Eric Olander


On Feb. 9, 2015, 10:37 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Feb. 9, 2015, 10:37 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 54755e8dd3f23ced313067566cd4ea867f8a496e 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 13, 2015, 12:22 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/tools/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  
core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
 PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
ac15d34425795d5be20c51b01fa1108bdcd66583 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 13, 2015, 12:31 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  
core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
 PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
ac15d34425795d5be20c51b01fa1108bdcd66583 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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


To make review easier, could you add the output of the command for all options 
for 2 consumer groups that consume 2 or more topics to the JIRA? It will make 
it easier to review. One thing to watch out for is the ease of scripting the 
output from this tool. I'd also suggest asking Clark/Tood or one of the SREs to 
review the output from the tool.


core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


--delete is sufficient. Same for the name of the corresponding variable in 
opts.



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


topic command => consumer command



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


If topic is not specified, why not delete information for all topics 
subscribed by this consumer group?



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


Invalid topic config?



core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala


It is better to rename this to DeleteConsumerGroupTest


- Neha Narkhede


On Jan. 13, 2015, 12:31 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 12:31 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   
> core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
>  PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 46
> > 
> >
> > --delete is sufficient. Same for the name of the corresponding variable 
> > in opts.

My only concern with --delete is that it doesn't indicate that this is deleting 
information for multiple groups.


- Onur


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


On Jan. 13, 2015, 12:31 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 12:31 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   
> core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
>  PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 46
> > 
> >
> > --delete is sufficient. Same for the name of the corresponding variable 
> > in opts.
> 
> Onur Karaman wrote:
> My only concern with --delete is that it doesn't indicate that this is 
> deleting information for multiple groups.

My understanding is that --delete will delete whatever groups are specified 
through --group. User should be able to specify multiple groups like this -

--group  --group  --delete


- Neha


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


On Jan. 13, 2015, 12:31 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 12:31 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   
> core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
>  PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 13, 2015, 6:36 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  
core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
 PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
ac15d34425795d5be20c51b01fa1108bdcd66583 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 46
> > 
> >
> > --delete is sufficient. Same for the name of the corresponding variable 
> > in opts.
> 
> Onur Karaman wrote:
> My only concern with --delete is that it doesn't indicate that this is 
> deleting information for multiple groups.
> 
> Neha Narkhede wrote:
> My understanding is that --delete will delete whatever groups are 
> specified through --group. User should be able to specify multiple groups 
> like this -
> 
> --group  --group  --delete

The following is what this script currently supports:

bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --list
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe --group 
foo
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe --group 
foo --config channelSocketTimeoutMs=400 --config channelRetryBackoffMsOpt=200
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
--delete-all-consumer-group-info-for-topic --topic topic1
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
--delete-all-consumer-group-info-for-topic --topic topic1 --force-delete

Maybe before proceeding, it would be best to iron out exactly what this script 
should support.


- Onur


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


On Jan. 13, 2015, 6:36 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   
> core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
>  PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 46
> > 
> >
> > --delete is sufficient. Same for the name of the corresponding variable 
> > in opts.
> 
> Onur Karaman wrote:
> My only concern with --delete is that it doesn't indicate that this is 
> deleting information for multiple groups.
> 
> Neha Narkhede wrote:
> My understanding is that --delete will delete whatever groups are 
> specified through --group. User should be able to specify multiple groups 
> like this -
> 
> --group  --group  --delete
> 
> Onur Karaman wrote:
> The following is what this script currently supports:
> 
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --list
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe 
> --group foo
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe 
> --group foo --config channelSocketTimeoutMs=400 --config 
> channelRetryBackoffMsOpt=200
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
> --delete-all-consumer-group-info-for-topic --topic topic1
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
> --delete-all-consumer-group-info-for-topic --topic topic1 --force-delete
> 
> Maybe before proceeding, it would be best to iron out exactly what this 
> script should support.

List and describe make sense, though for those, as I said earlier, it will be 
great to see sample output so we can review the format. 

For delete, there are 2 use cases -
1. Delete all consumer group information for the specified consumer groups
2. Delete consumer group information for the specified consumer groups for the 
specified topics only
3. It seems that you have a third option which is delete consumer group 
information for all consumer groups that have subscribed to a particular topic. 
I'm guessing the reason for including this option is to address deleted topics. 
Is that correct? 

For all delete options, would it be useful to also include a --dry-run option 
that outputs the information that the tool will delete if the --dry-run flag is 
removed?

Going forward, this tool should also support offset updates, though we can do 
that in a separate JIRA.


- Neha


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


On Jan. 13, 2015, 6:36 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   
> core/src/test/scala/unit/kafka/admin/DeleteAllConsumerGroupInfoForTopicInZKTest.scala
>  PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 46
> > 
> >
> > --delete is sufficient. Same for the name of the corresponding variable 
> > in opts.
> 
> Onur Karaman wrote:
> My only concern with --delete is that it doesn't indicate that this is 
> deleting information for multiple groups.
> 
> Neha Narkhede wrote:
> My understanding is that --delete will delete whatever groups are 
> specified through --group. User should be able to specify multiple groups 
> like this -
> 
> --group  --group  --delete
> 
> Onur Karaman wrote:
> The following is what this script currently supports:
> 
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --list
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe 
> --group foo
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe 
> --group foo --config channelSocketTimeoutMs=400 --config 
> channelRetryBackoffMsOpt=200
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
> --delete-all-consumer-group-info-for-topic --topic topic1
> bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
> --delete-all-consumer-group-info-for-topic --topic topic1 --force-delete
> 
> Maybe before proceeding, it would be best to iron out exactly what this 
> script should support.
> 
> Neha Narkhede wrote:
> List and describe make sense, though for those, as I said earlier, it 
> will be great to see sample output so we can review the format. 
> 
> For delete, there are 2 use cases -
> 1. Delete all consumer group information for the specified consumer groups
> 2. Delete consumer group information for the specified consumer groups 
> for the specified topics only
> 3. It seems that you have a third option which is delete consumer group 
> information for all consumer groups that have subscribed to a particular 
> topic. I'm guessing the reason for including this option is to address 
> deleted topics. Is that correct? 
> 
> For all delete options, would it be useful to also include a --dry-run 
> option that outputs the information that the tool will delete if the 
> --dry-run flag is removed?
> 
> Going forward, this tool should also support offset updates, though we 
> can do that in a separate JIRA.

Thanks for the feedback. I avoided providing sample output earlier because the 
commands aren't fully solidified yet. The three use cases seem reasonable, and 
yes that was the intent for (3).

The following is my plan for what gets deleted in each of the three use cases:
1. For each group g provided: Delete the entire /consumers/g directory.
2. Let's say we provide --topic t. For each group g provided:
  If the consumer group only consumes from topic t, wipe all of /consumers/g.
  Else only wipe /consumers/g/offsets/t and /consumers/g/owners/t.
3. Apply the same logic from 2 individually for each group associated with the 
given topic.

Here's my proposal for what the updated script would support:
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --list
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe --group 
foo
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --describe --group 
foo --config channelSocketTimeoutMs=400 --config channelRetryBackoffMsOpt=200
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete --group 
group1 --group group2
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete-for-topic 
--group group1 --group group2 --topic topic1
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
--delete-all-for-topic --topic topic1
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 
--delete-all-for-topic --topic topic1 --force-delete

--dry-run might also be useful. What kind of information do you think it should 
show? Maybe just list the directories that would be deleted?


- Onur


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


On Jan. 13, 2015, 6:36 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 15, 2015, 10:30 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
ac15d34425795d5be20c51b01fa1108bdcd66583 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 13, 2015, 5:36 a.m., Neha Narkhede wrote:
> > To make review easier, could you add the output of the command for all 
> > options for 2 consumer groups that consume 2 or more topics to the JIRA? It 
> > will make it easier to review. One thing to watch out for is the ease of 
> > scripting the output from this tool. I'd also suggest asking Clark/Tood or 
> > one of the SREs to review the output from the tool.

I just attached the output from a run through all the commands and options on 
the JIRA. I excluded the noise coming from SLF4J that I was getting on every 
command.


- Onur


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


On Jan. 15, 2015, 10:30 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 15, 2015, 10:30 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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


- Neha Narkhede


On Jan. 15, 2015, 10:30 p.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 15, 2015, 10:30 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

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

(Updated Jan. 22, 2015, 10:32 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Merged in work for KAFKA-1476 and sub-task KAFKA-1826


Diffs (updated)
-

  bin/kafka-consumer-groups.sh PRE-CREATION 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
28b12c7b89a56c113b665fbde1b95f873f8624a3 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
ac15d34425795d5be20c51b01fa1108bdcd66583 

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


Testing
---


Thanks,

Onur Karaman

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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


Onur, do you have an updated version of the console output from this tool?

- Neha Narkhede


On Jan. 22, 2015, 10:32 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 22, 2015, 10:32 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 23, 2015, 2:22 a.m., Neha Narkhede wrote:
> > Onur, do you have an updated version of the console output from this tool?

Hi Neha. I just uploaded it now.


- Onur


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


On Jan. 22, 2015, 10:32 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 22, 2015, 10:32 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Neha Narkhede]

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



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


As I described in my other comment, the only time safe enough to delete 
consumer group information is if there are no active consumers in the group. If 
not, then we should error out from --delete and give a WARNING to the user. 

The question is if --force-delete makes sense or not since that would mean 
deleting offsets and group information for active consumers, which is very 
disruptive.



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


How is this tool going to behave if the consumer's offset information is 
stored in kafka, not zookeeper?

The assumption of the user would be to handle that case transparently as 
well.



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


change to --delete



core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala


=> If set along with --delete

I'm not sure if I fully understood the purpose of force-delete. Basically, 
the only time safe for deleting a consumer group's offset information is if 
there are no live consumers in that group anymore. 

If so, --force-delete would mean deleting even if that is not true. This is 
pretty disruptive and I can't think of any case where this action will be 
useful. 

Thoughts?


- Neha Narkhede


On Jan. 22, 2015, 10:32 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 22, 2015, 10:32 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>

Re: Review Request 29831: Patch for KAFKA-1476

[Onur Karaman]

> On Jan. 23, 2015, 4:47 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala, line 259
> > 
> >
> > => If set along with --delete
> > 
> > I'm not sure if I fully understood the purpose of force-delete. 
> > Basically, the only time safe for deleting a consumer group's offset 
> > information is if there are no live consumers in that group anymore. 
> > 
> > If so, --force-delete would mean deleting even if that is not true. 
> > This is pretty disruptive and I can't think of any case where this action 
> > will be useful. 
> > 
> > Thoughts?

There are currently four types of delete:
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete --group g1 
--group g5
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete --group g3 
--group g4 --topic t2
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete --topic t1
bin/kafka-consumer-groups.sh --zookeeper 192.168.50.11:2181 --delete --topic t3 
--force-delete

--force-delete only applies to topic-wide delete. My concern was that when you 
do a topic-wide delete, you can potentially impact many consumer groups. So by 
default, topic-wide delete first checks if the topic still exists. My reasoning 
was that if a topic still exists during a topic-wide offset delete, it probably 
wasn't intentional. It overrides the default behavior by ignoring the 
topic-existance check.

One complication of the topic existance check is the following scenario:
1. We delete topic t.
2. t gets recreated due to some producers still producing events to t.
3. We try to do a topic-wide offset delete on t.
4. The check will prevent the offset delete from happening.

--force-delete attempts to address that scenario.

I agree that it is not safe to delete offsets while the group is active, and 
none of the 4 deletes currently check for this. While it is documented, it 
makes more sense to push this into the code.


- Onur


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


On Jan. 22, 2015, 10:32 a.m., Onur Karaman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29831/
> ---
> 
> (Updated Jan. 22, 2015, 10:32 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1476
> https://issues.apache.org/jira/browse/KAFKA-1476
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Merged in work for KAFKA-1476 and sub-task KAFKA-1826
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-groups.sh PRE-CREATION 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 28b12c7b89a56c113b665fbde1b95f873f8624a3 
>   core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> c14bd455b6642f5e6eb254670bef9f57ae41d6cb 
>   core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> ac15d34425795d5be20c51b01fa1108bdcd66583 
> 
> Diff: https://reviews.apache.org/r/29831/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Onur Karaman
> 
>