Skip to content

Conversation

@patelvp
Copy link
Contributor

@patelvp patelvp commented Feb 20, 2025

Issue: #2581

Scaling kafka consumers should be done in factors of partition count on the topic. This is to ensure that the partitions are evenly spread across all consumers. If the paritions are not evenly spread we run the risk of some partitions being consumed faster than other. This PR adds a new property on the kafka scaler ensureEvenDistributionOfPartitions. When this property is set to true the scaler ensure that the number of pods are always evenly spread across the number of topics on the partition.

Checklist

Fixes #2581

QA:
Tested this locally by pushing an image on a local kind cluster. Kafka consumer and producers are outside the cluster to get granual and quick control over production and consumption rate.
Plotted the pod count and kafka partition lag onto grafana.
Screenshot 2025-02-19 at 5 08 27 PM

@patelvp patelvp requested a review from a team as a code owner February 20, 2025 06:26
@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from a548ac9 to c46e703 Compare February 20, 2025 17:37
@JorTurFer
Copy link
Member

@dttung2905 @zroubalik , you are the Kafka experts xD
Does this make sense?

Copy link
Contributor

@dttung2905 dttung2905 left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR. Personally, I like the direction this PR is heading to. Just 1 small comment for my understanding

@zroubalik
Copy link
Member

zroubalik commented Feb 25, 2025

/run-e2e kafka
Update: You can check the progress here

@patelvp
Copy link
Contributor Author

patelvp commented Mar 3, 2025

@zroubalik Any thoughts on the PR?

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from c46e703 to a7ad359 Compare March 6, 2025 21:27
@patelvp
Copy link
Contributor Author

patelvp commented Mar 6, 2025

Force pushed a rebase

@dttung2905
Copy link
Contributor

dttung2905 commented Mar 8, 2025

/run-e2e kafka
Update: You can check the progress here

@zroubalik zroubalik mentioned this pull request Mar 26, 2025
40 tasks
@utkuaydn
Copy link

Would it be possible to add this functionality to the experimental kafka trigger as well?

@patelvp
Copy link
Contributor Author

patelvp commented Apr 16, 2025

Would it be possible to add this functionality to the experimental kafka trigger as well?

Yeah I could take a look at it.

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch 2 times, most recently from b18b0d1 to 56e8e43 Compare June 13, 2025 18:01
@rickbrouwer
Copy link
Member

rickbrouwer commented Jun 15, 2025

/run-e2e kafka
Update: You can check the progress here

@patelvp patelvp requested a review from zroubalik June 17, 2025 19:19
@patelvp
Copy link
Contributor Author

patelvp commented Jun 20, 2025

@rickbrouwer I don't see the logs for the failed test. Can you share how I can see them?

@rickbrouwer
Copy link
Member

rickbrouwer commented Jun 20, 2025

@rickbrouwer I don't see the logs for the failed test. Can you share how I can see them?

Of course!
You can find the log here:

https://github.com/kedacore/keda/actions/runs/15666250049/job/44131008541#step:9:1886

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from 56e8e43 to 41591d7 Compare August 11, 2025 01:01
@zroubalik zroubalik mentioned this pull request Aug 19, 2025
22 tasks
@patelvp
Copy link
Contributor Author

patelvp commented Aug 20, 2025

@rickbrouwer I don't see the logs for the failed test. Can you share how I can see them?

Of course! You can find the log here:

https://github.com/kedacore/keda/actions/runs/15666250049/job/44131008541#step:9:1886

@rickbrouwer The tests now pass with a rebase. Could it be possible it was a flake?
I can push a new rebase again

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.

@patelvp could you please fix conflicts and fix the static checks? I will trigger the e2e tests then.

Thanks!

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from 41591d7 to 2eef3fc Compare August 26, 2025 17:49
@patelvp patelvp requested a review from a team as a code owner August 26, 2025 17:49
@keda-automation keda-automation requested a review from a team August 26, 2025 17:50
@rickbrouwer
Copy link
Member

@patelvp there is still an issue in the static checks

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from 2eef3fc to ad2c2d9 Compare August 26, 2025 23:36
@rickbrouwer
Copy link
Member

rickbrouwer commented Aug 27, 2025

/run-e2e kafka
Update: You can check the progress here

@patelvp
Copy link
Contributor Author

patelvp commented Aug 27, 2025

Looking at the test failures

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from ad2c2d9 to 36603f3 Compare August 28, 2025 05:23
@patelvp
Copy link
Contributor Author

patelvp commented Aug 28, 2025

Okay, I tried the test locally. It took me a while to figure out that our corporate VPN was blocking some image pulls for the test to run correctly. The fix was an indentation issue in the test template!

@rickbrouwer
Copy link
Member

rickbrouwer commented Aug 28, 2025

/run-e2e kafka
Update: You can check the progress here

@patelvp
Copy link
Contributor Author

patelvp commented Aug 28, 2025

@rickbrouwer @zroubalik Should be good to review. Hoping I can make it in the next release so I don't have to keep maintaining this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for partition-based scaling to the Kafka scaler through a new ensureEvenDistributionOfPartitions property. When enabled, this feature ensures that the number of Kafka consumer pods aligns with factors of the topic's partition count to guarantee even partition distribution across consumers.

Key changes include:

  • Addition of the ensureEvenDistributionOfPartitions configuration option to both Kafka and Apache Kafka scalers
  • Implementation of mathematical logic to calculate appropriate scaling factors based on partition counts
  • Comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pkg/scalers/kafka_scaler.go Implements core logic for partition-based scaling and factor calculation
pkg/scalers/apache_kafka_scaler.go Adds the new configuration option to Apache Kafka scaler
pkg/scalers/kafka_scaler_test.go Adds comprehensive unit tests for metadata parsing and factor calculations
pkg/scalers/apache_kafka_scaler_test.go Updates test data structures to include new configuration option
tests/scalers/kafka/kafka_test.go Adds integration tests for the new scaling behavior
tests/scalers/apache_kafka/apache_kafka_test.go Adds integration tests for Apache Kafka scaler
CHANGELOG.md Documents the new feature addition

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@zroubalik
Copy link
Member

@rickbrouwer @zroubalik Should be good to review. Hoping I can make it in the next release so I don't have to keep maintaining this PR

@patelvp could you please check the review comments?

@dttung2905
Copy link
Contributor

yes. imo, once all the comments are addresssed, it should be good to merge

@zroubalik
Copy link
Member

@patelvp could you please resolve the comments so we can include this PR into the next release

@patelvp
Copy link
Contributor Author

patelvp commented Sep 30, 2025

@patelvp could you please resolve the comments so we can include this PR into the next release

I will get to this today

@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from 36603f3 to 1e8412a Compare October 1, 2025 15:54
Scaling kafka consumers should be done in factors of partition count on the topic. This is to ensure that the partitions are evenly spread across all consumers. If the paritions are not evenly spread we run the risk of some partitions being consumed faster than other. This PR adds a new property on the kafka scaler `ensureEvenDistributionOfPartitions`. When this property is set to true the scaler ensure that the number of pods are always evenly spread across the number of topics on the partition.

Signed-off-by: Vishal Patel <[email protected]>
@patelvp patelvp force-pushed the add-partition-based-scaling-kafka-scaler branch from 1e8412a to 315364c Compare October 1, 2025 16:02
@patelvp
Copy link
Contributor Author

patelvp commented Oct 1, 2025

@zroubalik Should be good now!

@rickbrouwer
Copy link
Member

rickbrouwer commented Oct 2, 2025

/run-e2e kafka
Update: You can check the progress here

@wozniakjan wozniakjan merged commit 64a3d8d into kedacore:main Oct 2, 2025
24 checks passed
alt-dima pushed a commit to alt-dima/keda that referenced this pull request Dec 13, 2025
…#6558)

Scaling kafka consumers should be done in factors of partition count on the topic. This is to ensure that the partitions are evenly spread across all consumers. If the paritions are not evenly spread we run the risk of some partitions being consumed faster than other. This PR adds a new property on the kafka scaler `ensureEvenDistributionOfPartitions`. When this property is set to true the scaler ensure that the number of pods are always evenly spread across the number of topics on the partition.

Signed-off-by: Vishal Patel <[email protected]>
Signed-off-by: Dmitriy Altuhov <[email protected]>
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Dec 22, 2025
…#6558)

Scaling kafka consumers should be done in factors of partition count on the topic. This is to ensure that the partitions are evenly spread across all consumers. If the paritions are not evenly spread we run the risk of some partitions being consumed faster than other. This PR adds a new property on the kafka scaler `ensureEvenDistributionOfPartitions`. When this property is set to true the scaler ensure that the number of pods are always evenly spread across the number of topics on the partition.

Signed-off-by: Vishal Patel <[email protected]>
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 Partitions Per Pod Scaling

7 participants