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

Remove duplicates from topics #2268

Closed
wants to merge 1 commit into from
Closed

Remove duplicates from topics #2268

wants to merge 1 commit into from

Conversation

Smriti-OSS
Copy link
Member

@Smriti-OSS Smriti-OSS commented Jul 14, 2022

This PR is intended to fix the issues observed in #2264.

Summary of the issue: Passing duplicate topics into the Consume function leads to uneven partition division between different consumers of the consumer group.

References: Java client handles this issue by always discarding duplicates - as it collapses the collection of topics into a set.

@ghost ghost added the cla-needed label Jul 14, 2022
@Smriti-OSS
Copy link
Member Author

@dnwe Could you please review the issue and PR.

@Smriti-OSS
Copy link
Member Author

Smriti-OSS commented Jul 19, 2022

Issue could be tested by adding this testcase to balance_strategy_test.go:

{
name: "2 members, 1 topic, 8 partitions each",
members: map[string][]string{"M1": {"T1", "T1", "T1", "T1", "T1", "T1", "T1", "T1"}, "M2": {"T1", "T1", "T1", "T1", "T1", "T1", "T1", "T1"}},
topics: map[string][]int32{"T1": {0, 1, 2, 3, 4, 5, 6, 7}},
expected: BalanceStrategyPlan{
"M1": map[string][]int32{"T1": {0, 1, 2, 3}},
"M2": map[string][]int32{"T1": {4, 5, 6, 7}},
},
},

This fails with:

=== RUN TestBalanceStrategyRange
=== RUN TestBalanceStrategyRange/2_members,_1_topic,_8_partitions_each#01
balance_strategy_test.go:101: Plan does not match expectation
expected: sarama.BalanceStrategyPlan{"M1":map[string][]int32{"T1":[]int32{0, 1, 2, 3}}, "M2":map[string][]int32{"T1":[]int32{4, 5, 6, 7}}}
actual: sarama.BalanceStrategyPlan{"M1":map[string][]int32{"T1":[]int32{0, 1, 2, 3, 4, 5, 6, 7}}}
--- FAIL: TestBalanceStrategyRange (0.00s)
--- FAIL: TestBalanceStrategyRange/2_members,_1_topic,_8_partitions_each#01 (0.00s)

@dnwe
Copy link
Collaborator

dnwe commented Jul 20, 2022

@Smriti-OSS ah thanks — yes so the issue is that the consumer has duplicate topics in the consumer group metadata sent as part of the JoinGroupRequest and used for generating the assignments

@dnwe
Copy link
Collaborator

dnwe commented Jul 20, 2022

I've pushed up #2285 with the sample unittest and to cover de-duplication in the balance_strategy — as I think it's worth doing it there as well as in the consumer group itself

@dnwe dnwe removed the cla-needed label Jul 12, 2023
@github-actions
Copy link

Thank you for your contribution! However, this pull request has not had any activity in the past 90 days and will be closed in 30 days if no updates occur.
If you believe the changes are still valid then please verify your branch has no conflicts with main and rebase if needed. If you are awaiting a (re-)review then please let us know.

@github-actions github-actions bot added the stale Issues and pull requests without any recent activity label Oct 10, 2023
@github-actions github-actions bot closed this Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues and pull requests without any recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants