kedacore / keda

KEDA is a Kubernetes-based Event Driven Autoscaling component. It provides event driven scale for any container running in Kubernetes
https://keda.sh
Apache License 2.0
8.29k stars 1.05k forks source link

Pulsar scaler for non-persistent topics? #6059

Closed kunwooy closed 1 week ago

kunwooy commented 1 month ago

Proposal

Currently the Pulsar scaler only supports persistent topics based on the topics' message backlog count. Since the backlog count is only available for persistent topics, non-persistent topics are currently not supported in the Pulsar scaler. Would it be possible to scale based on the number of unacknowledged messages in non-persistent topics?

Scaler Source

Pulsar

Scaling Mechanics

the number of unacknowledged messages on non-persistent topics.

Authentication Source

-

Anything else?

What does authentication source mean?

JorTurFer commented 1 month ago

It's a nice point. I have no idea about pulsar itself but including support for it'd be nice if it's possible. Are you willing to open a PR with the feature?

kunwooy commented 1 month ago

@JorTurFer Yes I do. I’ll open a PR.

kunwooy commented 1 month ago

@JorTurFer I have a question. Currently the name of the trigger for the Pulsar scaler is "msgBacklogThreshold" (https://keda.sh/docs/2.15/scalers/pulsar/#trigger-specification). I'm wondering if I should integrate this feature into the existing Pulsar scaler and use some common name for the trigger like "targetValue", or should just make another scaler for non-persistent topics.

JorTurFer commented 1 month ago

I'm wondering if I should integrate this feature into the existing Pulsar scaler and use some common name for the trigger like "targetValue", or should just make another scaler for non-persistent topics.

This'd be nice but we can't remove the old value directly as it's a breaking change. To remove the value we have to follow the deprecation policy -> https://github.com/kedacore/governance/blob/main/DEPRECATIONS.md#introducing-new-deprecations

I like the idea and from implementation pov it's just supporting both parameters during 2 versions and then removing the old one instead of just renaming it directly

kunwooy commented 1 month ago

@JorTurFer Just to be clear, you're suggesting the former approach where I introduce a new value while keeping the old "msgBacklogThreshold" for 2 versions following the deprecation policy, right?

Also, https://github.com/kedacore/keda/blob/66d4c9595e917a86b390bbed6eed88b1b7cc5c92/pkg/scalers/pulsar_scaler.go#L185 the comment here mentions that the former parameter name "msgBacklog" should've been removed in v2.14. Should I remove this too?

JorTurFer commented 1 month ago

yeah, that's the idea and yes please, remove it because we missed removing it during the proper version :(

I think that this line has to be removed too: image