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

Add support for partition based scaling on the kafka scaler #6558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
- **General**: Introduce new NSQ scaler ([#3281](https://github.com/kedacore/keda/issues/3281))
- **General**: Operator flag to control patching of webhook resources certificates ([#6184](https://github.com/kedacore/keda/issues/6184))
- **Azure Pipelines Scaler**: Introduce requireAllDemandsAndIgnoreOthers to match job demands while ignoring extras ([#5579](https://github.com/kedacore/keda/issues/5579))
- **Kafka Scaler**: Add support for even distribution of partitions to consumers ([#2581](https://github.com/kedacore/keda/issues/2581))

#### Experimental

Expand Down
66 changes: 63 additions & 3 deletions pkg/scalers/kafka_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import (
"context"
"errors"
"fmt"
"math"
"os"
"sort"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -68,8 +70,9 @@ type kafkaMetadata struct {

// If an invalid offset is found, whether to scale to 1 (false - the default) so consumption can
// occur or scale to 0 (true). See discussion in https://github.com/kedacore/keda/issues/2612
scaleToZeroOnInvalidOffset bool
limitToPartitionsWithLag bool
scaleToZeroOnInvalidOffset bool
limitToPartitionsWithLag bool
ensureEvenDistributionOfPartitions bool

// SASL
saslType kafkaSaslType
Expand Down Expand Up @@ -595,6 +598,22 @@ func parseKafkaMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger)
}
}

meta.ensureEvenDistributionOfPartitions = false
if val, ok := config.TriggerMetadata["ensureEvenDistributionOfPartitions"]; ok {
t, err := strconv.ParseBool(val)
if err != nil {
return meta, fmt.Errorf("error parsing ensureEvenDistributionOfPartitions: %w", err)
}
meta.ensureEvenDistributionOfPartitions = t

if meta.limitToPartitionsWithLag && meta.ensureEvenDistributionOfPartitions {
return meta, fmt.Errorf("limitToPartitionsWithLag and ensureEvenDistributionOfPartitions cannot be set simultaneously")
}
if len(meta.topic) == 0 && meta.ensureEvenDistributionOfPartitions {
return meta, fmt.Errorf("topic must be specified when using ensureEvenDistributionOfPartitions")
}
}

meta.version = sarama.V1_0_0_0
if val, ok := config.TriggerMetadata["version"]; ok {
val = strings.TrimSpace(val)
Expand Down Expand Up @@ -973,9 +992,15 @@ func (s *kafkaScaler) getTotalLag() (int64, int64, error) {
}
s.logger.V(1).Info(fmt.Sprintf("Kafka scaler: Providing metrics based on totalLag %v, topicPartitions %v, threshold %v", totalLag, len(topicPartitions), s.metadata.lagThreshold))

if !s.metadata.allowIdleConsumers || s.metadata.limitToPartitionsWithLag {
if !s.metadata.allowIdleConsumers || s.metadata.limitToPartitionsWithLag || s.metadata.ensureEvenDistributionOfPartitions {
// don't scale out beyond the number of topicPartitions or partitionsWithLag depending on settings
upperBound := totalTopicPartitions
// Ensure that the number of partitions is evenly distributed across the number of consumers
if s.metadata.ensureEvenDistributionOfPartitions {
nextFactor := getNextFactorThatBalancesConsumersToTopicPartitions(totalLag, totalTopicPartitions, s.metadata.lagThreshold)
s.logger.V(1).Info(fmt.Sprintf("Kafka scaler: Providing metrics to ensure even distribution of partitions on totalLag %v, topicPartitions %v, evenPartitions %v", totalLag, totalTopicPartitions, nextFactor))
totalLag = nextFactor * s.metadata.lagThreshold
}
if s.metadata.limitToPartitionsWithLag {
upperBound = partitionsWithLag
}
Expand All @@ -987,6 +1012,16 @@ func (s *kafkaScaler) getTotalLag() (int64, int64, error) {
return totalLag, totalLagWithPersistent, nil
}

func getNextFactorThatBalancesConsumersToTopicPartitions(totalLag int64, totalTopicPartitions int64, lagThreshold int64) int64 {
factors := FindFactors(totalTopicPartitions)
for _, factor := range factors {
if factor*lagThreshold >= totalLag {
return factor
}
}
return totalTopicPartitions
Comment on lines +1017 to +1022
Copy link
Contributor

Choose a reason for hiding this comment

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

Just trying to understand and confirm the logic here. Are we trying to get the smallest number of pods to satisfy the condition factor*lagThreshold >= totalLag ? The reason is the factors array is sorted in ascending order from FindFactors

Another follow up question is that what if it is sorted in descending order to provide a more aggressive strategy to reduce lag 🤔 ?

Copy link
Author

Choose a reason for hiding this comment

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

Just trying to understand and confirm the logic here. Are we trying to get the smallest number of pods to satisfy the condition factor*lagThreshold >= totalLag ? The reason is the factors array is sorted in ascending order from FindFactors

Yep just trying to find the smallest number of pods that will satisfy the condition

what if it is sorted in descending order to provide a more aggressive strategy to reduce lag

In that case, eventually, either we have to flip the conditional factor*lagThreshold >= totalLag to end up getting the same number of pods that we get with the proposed logic, or we risk running way more pods than we should ideally want. Consider if there are 100 partitions. It would immediately scale to 100 pods even for a lag of 100 and lagThreshold of 10.
Not sure if you I am missing anything when you say aggressive strategy?

}

type brokerOffsetResult struct {
offsetResp *sarama.OffsetResponse
err error
Expand Down Expand Up @@ -1052,3 +1087,28 @@ func (s *kafkaScaler) getProducerOffsets(topicPartitions map[string][]int32) (ma

return topicPartitionsOffsets, nil
}

// FindFactors returns all factors of a given number
func FindFactors(n int64) []int64 {
if n < 1 {
return nil
}

var factors []int64
sqrtN := int64(math.Sqrt(float64(n)))

for i := int64(1); i <= sqrtN; i++ {
if n%i == 0 {
factors = append(factors, i)
if i != n/i {
factors = append(factors, n/i)
}
}
}

sort.Slice(factors, func(i, j int) bool {
return factors[i] < factors[j]
})

return factors
}
Loading