KAFKA-7471: Multiple Consumer Group Management Feature by rootex- · Pull Request #5726 · apache/kafka

rootex- · 2018-10-02T15:14:35Z

FEATURE
KAFKA-7471 Multiple Consumer Group Management (Describe, Reset, Delete) + --groups-all option
KIP-379: Multiple Consumer Group Management
[DISCUSS]: KIP-379: Multiple Consumer Group Management

Description:

Describe/Delete/Reset offsets on multiple consumer groups at a time (including each group by repeating --group parameter)
Describe/Delete/Reset offsets on ALL consumer groups at a time (add new --all-groups option similar to --all-topics)
Reset plan CSV file generation reworked: structure updated to support multiple consumer groups and make sure that CSV file generation is done properly since there are no restrictions on consumer group names and symbols like commas and quotes are allowed.
Extending data output table format by adding "GROUP" column for all --describe queries

All Unit and integration tests implemented.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

…x_map_count bullet point

…e) + --groups-all option Functionality: * Describe/Delete/Reset offsets on multiple consumer groups at a time (including each group by repeating `--group` parameter) * Describe/Delete/Reset offsets on ALL consumer groups at a time (add new `--groups-all` option similar to `--topics-all`) * Export offsets to reset for multiple consumer groups to a CSV file (CSV generation export/import rework, CSV format rework)

…nt_Feature

…of github.com:rootex-/kafka into KAFKA-7471_Multiple_Consumer_Group_Management_Feature

hachikuji · 2018-10-29T18:39:14Z

@rootex- Thanks a lot for the contribution. I think this is a great idea. There are a few comments/suggestions in the KIP discussion thread. Can you have a look and let us know what you think?

rootex- · 2018-10-31T14:16:03Z

@rootex- Thanks a lot for the contribution. I think this is a great idea. There are a few comments/suggestions in the KIP discussion thread. Can you have a look and let us know what you think?

@hachikuji Hi Jason, thanks for messaging here. Missed the discussion thread without any email notifications. I'll check them out and leave my reply as soon as I can.

saagar2000 · 2018-11-09T16:37:44Z

@Rootex @hachikuji Can all consumer groups functionality extended to admin client ? i.e change listConsumeGroupOffsets to allow take multiple group ids and return all consumer group offsets or have listAllConsumerGroupOffsets method.

hachikuji · 2018-11-13T18:58:10Z

@saagar2000 Yeah, that's a good question It is a little curious that we made describeConsumerGroups a batched API, but we did not do the same for listConsumeGroupOffsets. I think the underlying reason is that the OffsetFetch API is not batched. We could consider changing it, of course. One awkward aspect of all the consumer group APIs is finding the right coordinators. So if you request offsets for two groups, you may end up sending two requests anyway because the coordinators may be on different brokers.

* Single group CSV format: "topic,partition,offset" * Multiple group CSV format: "group,topic,partition,offset"

…he GROUP column

…nt_Feature

rootex- · 2019-01-10T14:06:26Z

Please review the latest updates, according to [DISCUSS]: KIP-379: Multiple Consumer Group Management

https://github.com/rootex-/kafka into KAFKA-7471_Multiple_Consumer_Group_Management_Feature

vahidhashemian

@rootex- Thank you for submitting the PR. I left a few initial comments.
BTW, the KIP has enough binding votes for approval.

…g/Non-existing consumer groups

rootex- · 2019-01-25T14:12:50Z

one blank line is skipped.

Thanks, you're right once again. Previously, that preceding \n blank line before the Error message wasn't necessary since we expected --describe results from a single group. I also think it's a good idea.

The other cosmetic issue (misaligned column values) ...

Done, thanks! Check out, please

In the below example the last line seems to be unnecessary

I'll be back to this in a while

vahidhashemian

Thanks for addressing my earlier comments. I left a comment inline, regarding one of those comments.

vahidhashemian · 2019-02-02T07:13:29Z

Looks like there are build failures.

rootex- · 2019-02-02T09:32:41Z

@vahidhashemian Yes, there were lots of conflicts and tricky merge. Looks like this branch is too far away from the original trunk version causing troubles. I'll take time to take a closer look.

* Single group CSV format: "topic,partition,offset" * Multiple group CSV format: "group,topic,partition,offset"

…nt_Feature

rootex- · 2019-03-11T13:01:39Z

Looks like there are build failures.

Back to work. Fixed now 👌

What's going on with trunk branch? It looks broken and does not even compile 😯

rootex- · 2019-03-12T08:45:12Z

@vahidhashemian any further comments maybe?

rootex- · 2019-03-21T13:34:53Z

@vahidhashemian @hachikuji any further notes or corrections maybe? It's ready to be merged I believe 👌

rootex- · 2019-03-28T11:17:42Z

@vahidhashemian @hachikuji guys?

vahidhashemian · 2019-03-28T14:02:36Z

I'm on vacation right now. Will try to take a look within the next week or so.

vahidhashemian

Thanks for updating the PR and sorry for the delay in my review. I think it's close now. Left a few minor comments inline.

vahidhashemian · 2019-04-07T19:27:14Z

+    // Make sure we got a coordinator
+    TestUtils.waitUntilTrue(() => {
+      consumerGroupCommand.collectGroupState().coordinator.host() == "localhost"
+    }, "Can't find a coordinator


Was there a coordinator lookup issue that warranted this block? I couldn't get this test to fail without it.

Do not have any idea really, this code appeared after merge from trunk

Looks like the code has been committed by Gwen Shapira here to fix a test:
KAFKA-7937: Fix Flaky Test ResetConsumerGroupOffsetTest.testResetOffsetsNotExistingGroup (#6311)

@vahidhashemian is it ok if we leave it as is?

@vahidhashemian one more thing: I'm unable to compile my branch anymore after merging with trunk due to a bunch of missing classes in imports section of org.apache.kafka.common.protocol.ApiKeys.java class like:

import org.apache.kafka.common.message.CreateTopicsRequestData; import org.apache.kafka.common.message.CreateTopicsResponseData; import org.apache.kafka.common.message.DescribeGroupsRequestData; import org.apache.kafka.common.message.DescribeGroupsResponseData; import org.apache.kafka.common.message.ElectPreferredLeadersRequestData; import org.apache.kafka.common.message.ElectPreferredLeadersResponseData; import org.apache.kafka.common.message.LeaveGroupRequestData; import org.apache.kafka.common.message.LeaveGroupResponseData; import org.apache.kafka.common.message.MetadataRequestData; import org.apache.kafka.common.message.MetadataResponseData; import org.apache.kafka.common.message.SaslAuthenticateRequestData; import org.apache.kafka.common.message.SaslAuthenticateResponseData; import org.apache.kafka.common.message.SaslHandshakeRequestData; import org.apache.kafka.common.message.SaslHandshakeResponseData;

Those files are missing in the project.
And I'm also unable to compile trunk as well for the same reason.

Could we fix that?

Regarding the added block of code I though you added it since it appeared in the commit diff. I now remember Gwen adding that block, so all is good. Sorry for the confusion.

I think I also noticed the compile issues you are referring to. Will have to dig further to determine the root cause.

Otherwise, things look good to me. Thanks for removing the unused imports.

@vahidhashemian thanks for your accurate reviews, Vahid 👍 very straightforward and to the point

rootex- · 2019-04-10T13:26:51Z

@hachikuji @vahidhashemian according to Vahid, PR is ready to be merged. All improvements have been applied. Can't wait to start using it out-of-the-box 😄 Jason, your final word?

vahidhashemian

@hachikuji Did you also want to take a look at this PR before merging it?

hachikuji · 2019-04-15T23:19:53Z

@vahidhashemian I did a high level pass and it seems reasonable. If you're happy with it, I'd suggest merging.

ijuma · 2019-05-30T16:56:40Z

    compile project(':clients')
    compile libs.jacksonDatabind
+    compile libs.jacksonModuleScala
+    compile libs.jacksonDataformatCsv


@vahidhashemian I don't think we should have this dependency. We have avoided the Jackson module for Scala on purpose as it doesn't provide enough value. It would be good to fix this before 2.3.0 if at all possible. cc @hachikuji

To clarify, I'm referring to the Scala module (I got the line slightly wrong). The CSV one is probably fine.

How this was isolated:

comment out libs.jacksonModuleScala dependency in build.gradle

execute ./gradlew -PscalaVersion=2.12 jar

Result: two compile errors in kafka.admin.ConsumerGroupCommand (core module)

I volunteer to help with this.

@dejan2609 If you submit a PR, I'll help review it.

@ijuma Thanks for catching this. I wasn't aware of the limitation.
@dejan2609 Thanks for volunteering. If you like, you can open a JIRA for easier tracking of the issue.

@vahidhashemian Sure, I will open JIRA ticket.

@ijuma FYI: as a former Java developer that switched to build/release engineering I can't deliver quickly (but I assume this is not a post-haste).

JIRA ticket is created here: https://issues.apache.org/jira/browse/KAFKA-8466 Remove 'jackson-module-scala' dependency (and replace it with some code)

* Describe/Delete/Reset offsets on multiple consumer groups at a time (including each group by repeating `--group` parameter) * Describe/Delete/Reset offsets on ALL consumer groups at a time (add new `--all-groups` option similar to `--all-topics`) * Reset plan CSV file generation reworked: structure updated to support multiple consumer groups and make sure that CSV file generation is done properly since there are no restrictions on consumer group names and symbols like commas and quotes are allowed. * Extending data output table format by adding `GROUP` column for all `--describe` queries

Alexander Alexandrovich Dunayevsky and others added 14 commits December 26, 2017 15:52

KAFKA-6343 Documentation update in OS-level tuning section: add vm.ma…

88f171a

…x_map_count bullet point

KAFKA-6343 Documentation update in OS-level tuning section: add vm.ma…

6c4dd89

…x_map_count bullet point

KAFKA-6343 Update "File Descriptors" and "Memory Map Areas" sections

3796b01

Resolve conflicts

dcc3e9b

Merge remote-tracking branch 'upstream/trunk' into trunk

32e51b7

Merge remote-tracking branch 'upstream/trunk' into trunk

0b84102

KAFKA-6343 Fix indentation

f418995

Merge remote-tracking branch 'upstream/trunk' into trunk

83fcc7e

Merge branch 'trunk' of github.com:rootex-/kafka into trunk

cf0e7e1

Remove whitespaces

5948e84

Merge branch 'trunk' of https://github.com/apache/kafka into trunk

2e7b511

Merge branch 'trunk' of https://github.com/apache/kafka into trunk

f2c979c

Merge branch 'trunk' into KAFKA-7471_Multiple_Consumer_Group_Manageme…

e9496d3

…nt_Feature

rootex- changed the title ~~Kafka 7471 multiple consumer group management feature~~ KAFKA-7471 Multiple Consumer Group Management Feature Oct 2, 2018

rootex- changed the title ~~KAFKA-7471 Multiple Consumer Group Management Feature~~ KAFKA-7471: Multiple Consumer Group Management Feature Oct 2, 2018

rootex- and others added 2 commits October 3, 2018 17:24

KAFKA-7471: Minor cosmetic fix

8ba23be

Merge branch 'KAFKA-7471_Multiple_Consumer_Group_Management_Feature' …

21c4817

…of github.com:rootex-/kafka into KAFKA-7471_Multiple_Consumer_Group_Management_Feature

alex.dunayevsky and others added 4 commits January 8, 2019 22:16

Support both new and old CSV file formats to export/import offsets:

4847758

* Single group CSV format: "topic,partition,offset" * Multiple group CSV format: "group,topic,partition,offset"

KAFKA-7471 Minor code improvements

e9010c5

KAFKA-7471 All tables of --describe output format always start with t…

7104094

…he GROUP column

Merge branch 'trunk' into KAFKA-7471_Multiple_Consumer_Group_Manageme…

11b1d94

…nt_Feature

Alex Dunayevsky added 2 commits January 11, 2019 11:15

KAFKA-7471 Minor

f88b283

Merge branch 'KAFKA-7471_Multiple_Consumer_Group_Management_Feature' of

c003705

https://github.com/rootex-/kafka into KAFKA-7471_Multiple_Consumer_Group_Management_Feature

vahidhashemian reviewed Jan 16, 2019

View reviewed changes

Alex Dunayevsky added 5 commits January 25, 2019 11:01

KAFKA-7471 Minor impr

64cd61d

KAFKA-7471 Minor

b83982c

KAFKA-7471 Add preceding line before errors for Dead/Empty/Rebalancin…

4b20521

…g/Non-existing consumer groups

KAFKA-7471 Cosmetic fixes in formatting group members

4f8d5ca

KAFKA-7471 Minor

40e73a5

KAFKA-7471 A few issues with naming

1d243d4

vahidhashemian reviewed Jan 30, 2019

View reviewed changes

Comment thread core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala Outdated

KAFKA-7471 Remove caption printing for empty assignments

1508b38

alex.dunayevsky added 2 commits March 7, 2019 15:17

Support both new and old CSV file formats to export/import offsets:

3c97939

* Single group CSV format: "topic,partition,offset" * Multiple group CSV format: "group,topic,partition,offset"

KAFKA-7471 Default sorting of --describe output by consumer group.id

9bb66ff

rootex- force-pushed the KAFKA-7471_Multiple_Consumer_Group_Management_Feature branch from a15861b to 9bb66ff Compare March 11, 2019 12:19

Merge branch 'trunk' into KAFKA-7471_Multiple_Consumer_Group_Manageme…

5623864

…nt_Feature

KAFKA-7471 Merge updates

f5fc915

vahidhashemian reviewed Apr 7, 2019

View reviewed changes

KAFKA-7471 Remove unused imports

d46a39a

vahidhashemian approved these changes Apr 15, 2019

View reviewed changes

vahidhashemian merged commit 47a9871 into apache:trunk Apr 15, 2019

dejan2609 mentioned this pull request May 30, 2019

KAFKA-7197 expand gradle build: include Scala 2.13 #5454

Merged

ijuma reviewed May 30, 2019

View reviewed changes

Conversation

rootex- commented Oct 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

hachikuji commented Oct 29, 2018

Uh oh!

rootex- commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saagar2000 commented Nov 9, 2018

Uh oh!

hachikuji commented Nov 13, 2018

Uh oh!

rootex- commented Jan 10, 2019

Uh oh!

vahidhashemian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rootex- commented Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vahidhashemian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vahidhashemian commented Feb 2, 2019

Uh oh!

rootex- commented Feb 2, 2019

Uh oh!

rootex- commented Mar 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rootex- commented Mar 12, 2019

Uh oh!

rootex- commented Mar 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rootex- commented Mar 28, 2019

Uh oh!

vahidhashemian commented Mar 28, 2019

Uh oh!

vahidhashemian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rootex- Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vahidhashemian Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rootex- commented Apr 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vahidhashemian left a comment

Choose a reason for hiding this comment

Uh oh!

hachikuji commented Apr 15, 2019

Uh oh!

Choose a reason for hiding this comment

rootex- commented Oct 2, 2018 •

edited

Loading

rootex- commented Oct 31, 2018 •

edited

Loading

rootex- commented Jan 25, 2019 •

edited

Loading

rootex- commented Mar 11, 2019 •

edited

Loading

rootex- commented Mar 21, 2019 •

edited

Loading

rootex- Apr 8, 2019 •

edited

Loading

vahidhashemian Apr 8, 2019 •

edited

Loading

rootex- commented Apr 10, 2019 •

edited

Loading