KAFKA-8538 (part of KIP-345): add group.instance.id to DescribeGroup#6957
Merged
guozhangwang merged 5 commits intoapache:trunkfrom Jun 28, 2019
Merged
KAFKA-8538 (part of KIP-345): add group.instance.id to DescribeGroup#6957guozhangwang merged 5 commits intoapache:trunkfrom
guozhangwang merged 5 commits intoapache:trunkfrom
Conversation
abbccdda
commented
Jun 17, 2019
Author
There was a problem hiding this comment.
@hachikuji @guozhangwang I noticed that this class was only used in the deprecated admin client, do we still need to add group.instance.id support to it? If not, I could consider removing the follow-up changes in scala code.
Contributor
There was a problem hiding this comment.
I think @ijuma already have a PR for removing the deprecated admin clients. We can wait for his PR to be merged first?
abbccdda
force-pushed
the
describe_instance_id
branch
2 times, most recently
from
4f30df9 to
a855838
Compare
abbccdda
changed the title
abbccdda
force-pushed
the
describe_instance_id
branch
from
a855838 to
f9fb070
Compare
Author
|
retest this please |
Contributor
There was a problem hiding this comment.
I think @ijuma already have a PR for removing the deprecated admin clients. We can wait for his PR to be merged first?
Contributor
There was a problem hiding this comment.
Actually the constructor of all such description classes can just be default package-private since they are only used by KafkaAdminClient, and hence can just be private APIs, and we do not need to deprecate-overload any more.
ijuma
reviewed
Jun 21, 2019
abbccdda
force-pushed
the
describe_instance_id
branch
from
f9fb070 to
3a7aee6
Compare
Contributor
guozhangwang
left a comment
guozhangwang
left a comment
There was a problem hiding this comment.
LGTM except one minor comment, ping another pair of eyes before merging.
| private final MemberAssignment assignment; | ||
|
|
||
| public MemberDescription(String memberId, String clientId, String host, MemberAssignment assignment) { | ||
| public MemberDescription(String memberId, |
Contributor
There was a problem hiding this comment.
As mentioned above, we can make this constructor default access.
Author
|
Retest this please |
1 similar comment
Author
|
Retest this please |
Author
|
@ableegoldman @vvcephei Could you also take a look? |
Author
|
@ableegoldman Thank you! |
Author
|
Retest this please |
hachikuji
pushed a commit
that referenced
this pull request
Jul 12, 2019
This patch fixes a compatibility breaking `MemberDescription` constructor change in #6957. It also updates `equals` and `hashCode` for the new `groupInstanceId` field that was added in the same patch. Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
bfncs
pushed a commit
to bfncs/kafka
that referenced
this pull request
Jul 12, 2019
) This patch fixes a compatibility breaking `MemberDescription` constructor change in apache#6957. It also updates `equals` and `hashCode` for the new `groupInstanceId` field that was added in the same patch. Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
hachikuji
pushed a commit
that referenced
this pull request
May 27, 2020
#7886) Kafka-8538(#6957) has already added `group.instance.id` to `MemberDescription` but didn't print it in the describe group output, so this patch adds the logic to do so. Before the change, the describe command prints as follows: ``` GROUP CONSUMER-ID HOST CLIENT-ID #PARTITIONS DemoConsumer consumer-DemoConsumer-2-89251f12-f0ae-4dc1-a118-bda49f2a6e86 /127.0.0.1 consumer-DemoConsumer-2 0 DemoConsumer consumer-DemoConsumer-1-72221c6b-f3d9-4c68-96db-ffffa12ddf93 /127.0.0.1 consumer-DemoConsumer-1 1 ``` After the change, the describe command prints as follows: ``` GROUP CONSUMER-ID GROUP-INSTANCE-ID HOST CLIENT-ID #PARTITIONS DemoConsumer groupIns2-f050379c-9c0d-433c-bbe0-44de6177b60d groupIns2 /127.0.0.1 consumer-DemoConsumer-groupIns2 0 DemoConsumer groupIns1-44805ba9-ae6f-49d3-89af-44a4b95aff8d groupIns1 /127.0.0.1 consumer-DemoConsumer-groupIns1 1 ``` If all the `GROUP-INSTANCE-ID` is null, just as the previous: ``` GROUP CONSUMER-ID HOST CLIENT-ID #PARTITIONS DemoConsumer consumer-DemoConsumer-2-89251f12-f0ae-4dc1-a118-bda49f2a6e86 /127.0.0.1 consumer-DemoConsumer-2 0 DemoConsumer consumer-DemoConsumer-1-72221c6b-f3d9-4c68-96db-ffffa12ddf93 /127.0.0.1 consumer-DemoConsumer-1 1 ``` Reviewers: Alice <WheresAlice@users.noreply.github.com>, Matthias J. Sax <matthias@confluent.io>, Boyang Chen <boyang@confluent.io>, Jason Gustafson <jason@confluent.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Include group.instance.id in the describe group result for better visibility.
Committer Checklist (excluded from commit message)