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

[server] Avoid race condition between pick consumer call and actual consumer subscribe API #1301

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

sixpluszero
Copy link
Contributor

[server] Avoid race condition between pick consumer call and actual consumer subscribe API

Current implementation does not cover the race condition case between pick consumer API call and actual consumer subscribe call. So the assignment size can be inaccurate during calculation. To address this issue, this PR adds maps to keep track of the basic load and store level load for each consumer, and update it during pickConsumerForPartition() API

How was this PR tested?

Adjust unit test to more coverage.

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

Copy link
Contributor

@gaojieliu gaojieliu left a comment

Choose a reason for hiding this comment

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

This code change is fine overall, but it is not fully accurate as unsubscription can happen concurrently with pickConsumerForPartition call.
Since pickConsumerForPartition function is tracking these maps syncly, it can guarantee at least the even distribution for the same resource, which is potentially good enough.

If ^ is a correct understanding, can you convert it into javadoc to avoid confusion.

@sixpluszero sixpluszero merged commit 37a6951 into linkedin:main Nov 13, 2024
45 checks passed
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.

2 participants