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

Pulsar scaler for non-persistent topics? #6059

Closed
kunwooy opened this issue Aug 10, 2024 · 6 comments · Fixed by #6092
Closed

Pulsar scaler for non-persistent topics? #6059

kunwooy opened this issue Aug 10, 2024 · 6 comments · Fixed by #6092

Comments

@kunwooy
Copy link
Contributor

kunwooy commented Aug 10, 2024

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
Copy link
Member

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
Copy link
Contributor Author

kunwooy commented Aug 12, 2024

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

@JorTurFer JorTurFer assigned JorTurFer and kunwooy and unassigned JorTurFer Aug 13, 2024
@kunwooy
Copy link
Contributor Author

kunwooy commented Aug 15, 2024

@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
Copy link
Member

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
Copy link
Contributor Author

kunwooy commented Aug 15, 2024

@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,

// FIXME: msgBacklog support DEPRECATED to be removed in v2.14
the comment here mentions that the former parameter name "msgBacklog" should've been removed in v2.14. Should I remove this too?

@JorTurFer
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready To Ship
Development

Successfully merging a pull request may close this issue.

2 participants