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

fix(consumer): loopCheckPartitionNumbers may not cause rebalance after adding new partitions #2539

Closed

Conversation

napallday
Copy link
Contributor

fix #2460

…ew partitions

Signed-off-by: napallday <bzx0619@gmail.com>
@napallday napallday force-pushed the napallday/fix-loopCheckPartitionNumbers branch from d479e1c to 51ff5be Compare August 1, 2023 15:04
@napallday napallday changed the title fix: loopCheckPartitionNumbers may not cause rebalance after adding n… fix(consumer): loopCheckPartitionNumbers may not cause rebalance after adding new partitions Aug 1, 2023
@napallday
Copy link
Contributor Author

napallday commented Aug 1, 2023

I know there is a data race in the newly-added test file

func (h *loopCheckerHandler) Setup(s ConsumerGroupSession) error {
	if len(s.Claims()["my-topic"]) == 1 {
		if err := h.group.client.RefreshMetadata("my-topic"); err != nil {
			h.Error(err)
		}
		// change RefreshFrequency so that loopCheckPartitionNumbers won't return directly
***		h.group.client.(*nopCloserClient).Client.(*client).conf.Metadata.RefreshFrequency = 5 * time.Millisecond
		go h.group.loopCheckPartitionNumbers([]string{"my-topic"}, s.(*consumerGroupSession))
	} else {
		h.cancel()
	}
	return nil
}

The added test will fail in the old versions without a race detector, but I haven't managed to write a test to reproduce the issue without a data race.

If it's difficult to resolve the data race, how about we just remove the unit test if the changes look good to you guys? @dnwe @hindessm

@napallday
Copy link
Contributor Author

I found another critical bug in our company, which is related to this issue.
I'll open another MR to fix both of them.

@napallday napallday closed this Aug 4, 2023
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.

loopCheckPartitionNumbers function some problem
1 participant