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

Roll controller nodes on num.partitions change #10594

Merged

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Sep 17, 2024

Type of change

  • Bugfix

Description

Currently, we do not roll the controller-only nodes when num.partitions option changes. But this option seems to be used by the controller nodes and not brokers. So any changes to it are ignored unless the controllers are rolled. This PR adds it to the list of controller relevant options, so that we roll the controllers when it is changed.

This was raised up in https://cloud-native.slack.com/archives/CMH3Q3SNP/p1726561281223539

Checklist

  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally

Signed-off-by: Jakub Scholz <www@scholzj.com>
@scholzj scholzj added this to the 0.44.0 milestone Sep 17, 2024
@scholzj scholzj requested a review from ppatierno September 17, 2024 13:36
@scholzj scholzj changed the title Roll controller nodes on num.partitions change Roll controller nodes on num.partitions change Sep 17, 2024
@katheris
Copy link
Contributor

The change looks good to me, but tagging @showuon who reviewed the list when it was initially added for awareness.

@scholzj
Copy link
Member Author

scholzj commented Sep 17, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@showuon showuon 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 the fix!
I will re-look at the list again when available.

@scholzj
Copy link
Member Author

scholzj commented Sep 18, 2024

I will re-look at the list again when available.

@showuon Not sure it is worth the time. Hopefully, this will finally work properly in Kafka 3.9 and we will be able to get rid of the list.

@showuon
Copy link
Member

showuon commented Sep 18, 2024

Hopefully, this will finally work properly in Kafka 3.9 and we will be able to get rid of the list.

I thought we still need it after kafka 4.0 (without ZK) because we need to know if the changed configs need to roll controller or not. Why could we get rid of the list?

@scholzj
Copy link
Member Author

scholzj commented Sep 18, 2024

I thought that being able to talk to controllers should be enough for that. But maybe I'm wrong. @tinaselenge might know better.

@scholzj scholzj merged commit b926c5c into strimzi:main Sep 18, 2024
21 checks passed
@scholzj scholzj deleted the roll-controllers-on-num-partitions-change branch September 18, 2024 10:50
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.

5 participants