-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
GCP PubSub activationValue not working correctly #5383
Comments
Hello, |
Could you clarify this a bit @JorTurFer ? If the default was 1 and it now used the updated logic why would we need to set it to two and not just use the default? Either way reading the docs and how I and my team understood this value is "active the scaler when this number is hit" but the logic backing this at the moment doesn't line up. I think it either needs to be made more clear in the docs or changes need to be made here |
+1 that The docs only state that the default is 0, I can't find anywhere that specifies this is a ">" rather than the ">=" you would expect based on the name. Eg:
is currently the most detailed explanation of the activation threshold in the docs, which is not very specific. Updating the docs at least would make this significantly less likely to confuse people. |
Because if the logic is
Let's update docs to be more clear. We are happy improving our docs as much as possible. Changing the current behaviour isn't possible as it would be a breaking change (a user with |
No we want to activate it when we get at least 1 message which is why we set the activationValue to 1; assuming it meant (as the name implies) active the scaler when the value hits this number. Just without reading the docs the assumption is built into the name itself. I don't mind updating the docs and this likely doesn't matter in 99% of the cases but does for us since this is only active in our dev environments where having 0/1 message makes a giant difference. I'd still advocate for updating the logic here though; Having it use 1 as the default and using |
This is default behaviour, the scaler is active it there is at least 1 message and not active if there isn't any message
IMHO, this can't be done because it changes the current behaviour and we aren't allowed by our deprecation policy. Despite our deprecation policy, I don't think either that this change is required and docs seems the best place for explaining this. The last but not least, there are other 60 scalers with the same logic which need to be updated as well if we agree with changing the logic from @zroubalik , @tomkerkhove , WDYT about this? |
I understand the confusion on this, but I would like to avoid changing the behavior. It is practially a breaking change for 65+ different scalers. The behavior you would like to achieve can be easily done by setting activationValue to 0 (or omitting this setting completely). What we could definitely do is to update docs to be more clear what the activation is about. |
Thanks @zroubalik @JorTurFer . I've gone ahead and updated the docs to hopefully make this clearer |
Report
In the docs for the scaler the
activationValue
is activationValue - Target value for activating the scaler.This reads as the value at which the scaler should become active. In practice it appears that the metric value has to be greater than this value and not equal to.
Where we use KEDA is mostly in our development environments where we don't have many tasks in pub sub and we've set this activationValue to 1 by default. For queues that only get a single task every now and again we notice this is not activating the scaler and instead will only do so when there are at least 2 messages
Expected Behavior
Setting the activationValue should activate the scaler when the metric hits this number
Actual Behavior
Setting the activationValue activates the scaler only when it exceeds this value
Steps to Reproduce the Problem
Logs from KEDA operator
KEDA Version
2.13.0
Kubernetes Version
1.27
Platform
Google Cloud
Scaler Details
Google Cloud Platform Pub/Sub
Anything else?
It looks like this is where the issue lies - https://github.com/kedacore/keda/blame/main/pkg/scalers/gcp_pubsub_scaler.go#L197
I believe the comparison here should be a
>=
instead of just a>
The text was updated successfully, but these errors were encountered: