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

Kafka: Ability to set upper bound to the number of partitions with lag #4453

Closed

Conversation

DmitrySenin
Copy link
Contributor

@DmitrySenin DmitrySenin commented Apr 13, 2023

Added a way to configure the Kafka scaler to limit the max number of spun-up replicas to the number of partitions with non-zero lag.
This flag is definitely not compatible with allowIdleConsumers. Hence, added the respective validation
Also I don't see much sense in not having a topic when using the new setting. So added a validation check for this case as well

Checklist

  • Tests have been added
  • Changelog has been updated and is aligned with our changelog requirements
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #3997

Relates to kedacore/keda-docs#1107

Signed-off-by: Dmitrii Senin <white.lord.mane@gmail.com>
Signed-off-by: Dmitrii Senin <white.lord.mane@gmail.com>
Signed-off-by: Dmitrii Senin <white.lord.mane@gmail.com>
@DmitrySenin DmitrySenin requested a review from a team as a code owner April 13, 2023 12:17
@DmitrySenin DmitrySenin changed the title Feature/kafka active partitions Ability to set upper bound to the number of partitions with lag Apr 13, 2023
Signed-off-by: Dmitrii Senin <white.lord.mane@gmail.com>
@DmitrySenin
Copy link
Contributor Author

I would need to some help with testing. Do I need to test all locally? Or there is a test env(with a Kafka scaler) which gets automatically spun up?

@zroubalik
Copy link
Member

I would need to some help with testing. Do I need to test all locally? Or there is a test env(with a Kafka scaler) which gets automatically spun up?

There are e2e tests that you can run locally (against k8s cluster) and then we can also trigger them on the PR:
https://github.com/kedacore/keda/blob/main/tests

You should extend this test: https://github.com/kedacore/keda/blob/main/tests/scalers/kafka/kafka_test.go

@zroubalik
Copy link
Member

any update on this please?

@DmitrySenin
Copy link
Contributor Author

I've been busy at work. I should be back with e2e tests by Monday

@zroubalik
Copy link
Member

I've been busy at work. I should be back with e2e tests by Monday

gentle ping :)

@zroubalik zroubalik changed the title Ability to set upper bound to the number of partitions with lag Kafka: Ability to set upper bound to the number of partitions with lag May 18, 2023
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

To fix the Static Checks, please add exception rule here: https://github.com/kedacore/keda/blob/main/.golangci.yml#L53

@BojanZelic
Copy link
Contributor

@DmitrySenin any update on this feature? I could give implementing the tests & anything left over a try to get this merged; Would love to see this implemented

@DmitrySenin
Copy link
Contributor Author

@BojanZelic , I'm unlikely to get back to this one in the next two months. So feel free to pick up.
I think Static Checks need to be fixed and unit tests need to be added

@zroubalik
Copy link
Member

Resolved in #5060

@zroubalik zroubalik closed this Dec 21, 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.

Kafka scaler: limit number of replicas to the number of partitions with lag.
3 participants