Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Consumer Group to use Client #947

Merged
merged 2 commits into from
Sep 12, 2022

Conversation

rhansen2
Copy link
Collaborator

@rhansen2 rhansen2 commented Jul 8, 2022

No description provided.

@rhansen2 rhansen2 changed the base branch from main to 0.5 July 8, 2022 20:38
@rhansen2 rhansen2 marked this pull request as ready for review July 29, 2022 02:27
Copy link
Contributor

@achille-roussel achille-roussel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed the PR during our team meeting today and agreed on making the following changes:

  • base the change on the 0.4 branch since it contains minor modifications to the exported APIs (removed an unused field + change fields from int32 to int types)
  • explore passing a parent context to the methods of the coordinator interface
  • add the AllowAutoTopicCreation field to the top level MetadataRequest type so we don't need to bypass the abstraction layers

reader.go Outdated Show resolved Hide resolved
consumergroup.go Outdated Show resolved Hide resolved
Copy link
Contributor

@achille-roussel achille-roussel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking ready to merge in my opinion 🙌

@achille-roussel achille-roussel merged commit ee37c7f into segmentio:main Sep 12, 2022
@rhansen2 rhansen2 deleted the consumer-group-client branch October 21, 2022 18:38
@moolitayer
Copy link

Hi @rhansen2 @achille-roussel,

Thank you for working on this awsome library!

Recently we've seen an issue on upgrading from v0.4.35 to v0.4.36. The issue went away after downgrad:

Failed to join X: [79] Member ID Required: the group member needs to have a valid member id before actually entering a consumer group
Failed to join group X: kafka.(*Client).JoinGroup: unexpected EOF
Failed to sync group X: kafka.(*Client).SyncGroup: unexpected EOF
Failed to sync group X: [27] Rebalance In Progress: the coordinator has begun rebalancing the group, the client should rejoin the group
kafka.(*Client).SyncGroup: unexpected EOF

Do you think it might be ralted to this change? Should I create an issue for it with our env details + code?

@rhansen2
Copy link
Collaborator Author

Could be related. Please open an issue and we'll take a look, thanks!

@jhartzell
Copy link

Hi @rhansen2 @achille-roussel,

Thank you for working on this awsome library!

Recently we've seen an issue on upgrading from v0.4.35 to v0.4.36. The issue went away after downgrad:

Failed to join X: [79] Member ID Required: the group member needs to have a valid member id before actually entering a consumer group
Failed to join group X: kafka.(*Client).JoinGroup: unexpected EOF
Failed to sync group X: kafka.(*Client).SyncGroup: unexpected EOF
Failed to sync group X: [27] Rebalance In Progress: the coordinator has begun rebalancing the group, the client should rejoin the group
kafka.(*Client).SyncGroup: unexpected EOF

Do you think it might be ralted to this change? Should I create an issue for it with our env details + code?

Also seeing the same issue under a Strimzi cluster

@rhansen2
Copy link
Collaborator Author

rhansen2 commented Nov 2, 2022

@moolitayer or @jhartzell Are either of you able to create an issue with a reproduction of the issues you're seeing?

@rhansen2
Copy link
Collaborator Author

rhansen2 commented Nov 2, 2022

@moolitayer @jhartzell
While not exactly what you're seeing, I did find a potential source of unexpected EOFs. Would you be able to try out https://github.com/rhansen2/kafka-go/tree/join-group-meta-parsing and see if that changes anything for you?

Thanks!

rhansen2 added a commit to rhansen2/kafka-go that referenced this pull request Nov 9, 2022
rhansen2 added a commit that referenced this pull request Nov 9, 2022
* Revert "Fixes for consumer group (#1022)"

This reverts commit a5f270d.

* Revert "Refactor Consumer Group to use Client (#947)"

This reverts commit ee37c7f.
@rhansen2 rhansen2 restored the consumer-group-client branch November 9, 2022 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants