Skip to content

KAFKA-19276: Trigger rebalance for streams config update #19967

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
Open

KAFKA-19276: Trigger rebalance for streams config update #19967

wants to merge 10 commits into from

Conversation

UladzislauBlok
Copy link
Contributor

@UladzislauBlok UladzislauBlok commented Jun 15, 2025 •
edited
Loading

Create listener to react on config update.

Implementation was proposed by lucasbru. What was done:

  1. Add a new default method onGroupConfigChanged to Group. By default he's doing nothing
  2. Implement it in StreamsGroup to set an internal flag to trigger a config-related rebalance
  3. Add a method to GroupMetadataManager to call onGroupConfigChanged for the right group.
  4. Hook up GroupCoordinatorService and GroupConfigManager so that the latter calls the former when a group config changes.
  5. GroupCoordinatorService calls all the GroupCoordinatorShard's under the hood, then GroupCoordinatorShard propagate this call to the underline GroupMetadataManager

@github-actions github-actions bot added the triage PRs from the community label Jun 15, 2025
@UladzislauBlok UladzislauBlok changed the title KAFKA-19276: Trigger rebalance for streams config updat [WIP] KAFKA-19276: Trigger rebalance for streams config update [WIP] Jun 15, 2025
@github-actions GitHub Actions
Copy link

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

@UladzislauBlok
Copy link
Contributor Author

@lucasbru Hey Lucas. This is my proposed solution for KAFKA-19276. Could you please check the idea, and if all good I'll add tests and java docs

@lucasbru
Copy link
Member

Hey @UladzislauBlok . Thanks for your contribution! With the 4.1 release, I did not get to review this - but I will!

@github-actions GitHub Actions
Copy link

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

lucasbru
lucasbru reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@lucasbru lucasbru left a comment

Choose a reason for hiding this comment

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

I left a first round of comments to better undestand this PR

@github-actions github-actions bot removed needs-attention triage PRs from the community labels Jul 8, 2025
lucasbru
lucasbru reviewed Jul 8, 2025
View reviewed changes
Copy link
Member

@lucasbru lucasbru left a comment

Choose a reason for hiding this comment

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

Thanks for you updated. I left some further comments. Not sure we have quite nailed the design yet.

@github-actions github-actions bot removed the small Small PRs label Jul 14, 2025
@UladzislauBlok UladzislauBlok changed the title KAFKA-19276: Trigger rebalance for streams config update [WIP] KAFKA-19276: Trigger rebalance for streams config update Jul 17, 2025
lucasbru
lucasbru reviewed Jul 21, 2025
View reviewed changes
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java Outdated
@@ -272,6 +272,8 @@ public GroupCoordinatorShard build() {
.withAuthorizerPlugin(authorizerPlugin)
.build();

groupConfigManager.registerListener(groupMetadataManager);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will work. There is only a single groupConfigManager, but there is one group metadata manager per metrics shared. I think we have to register the group coordinator service as a listener.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, didn't know about that. I'll prepare fix shortly
btw. any suggestion how to learn internal kafka design? Any good knowledge source?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think there is anything out htere that is kept up-to-date. There is only reading the code. I also did not know how many instances of GroupConfigManager there are, I followed the code.

@UladzislauBlok UladzislauBlok requested a review from lucasbru July 28, 2025 18:08
@lucasbru
Copy link
Member

lucasbru commented Aug 1, 2025

@UladzislauBlok thanks for the updates. This is looking good, unfortunately I don't have time for a final review today and leaving for PTO. This will have to wait until I come back - just letting you know that I haven't forgotten about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasbru lucasbru Awaiting requested review from lucasbru

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
group-coordinator streams
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@UladzislauBlok @lucasbru