-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Fix KEDA Query to Use executor Field Instead of queue for Multiple Executors #52840
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
base: main
Are you sure you want to change the base?
Fix KEDA Query to Use executor Field Instead of queue for Multiple Executors #52840
Conversation
Tests need to be fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes! Could you please rebase it before merging it? The PR has 137 commits
I think you are merging the changes. You need to rebase from master and force push changes to your branch. |
- Update workers.keda.query in values.yaml to check executor attribute
- Modified the KEDA autoscaling query in values.schema.json to handle KubernetesExecutor separately from CeleryKubernetesExecutor, ensuring tasks with KubernetesExecutor are filtered by executor type instead of queue name. - Updated the corresponding logic in test_keda.py to reflect the new query structure, improving test accuracy for KubernetesExecutor scenarios.
b749ba9
to
15b0e67
Compare
I think I changed it correctly this time! Thanks a lot for the help! |
This is it, thanks! :) |
Description:
When multiple executors (e.g.,
CeleryExecutor,KubernetesExecutor
) are configured, the worker defaultKEDA
query uses thequeue
column in the Airflow metadata database to determine Celery worker scaling.However, task routing in Airflow is based on the
executor
attribute, notqueue
. This causes Celery workers to scale up unnecessarily for tasks explicitly configured withexecutor='KubernetesExecutor'
but using thedefault
queueChanges:
This PR updates the KEDA query logic in
values.yaml
to correctly filter tasks based on the executor field when multipleexecutors
are configured:CeleryKubernetesExecutor
, the existing logic is retained, usingqueue != '{{ .Values.config.celery_kubernetes_executor.kubernetes_queue }}'
.KubernetesExecutor
(alone or withCeleryExecutor
), the query is updated to useexecutor != 'KubernetesExecutor'
ensuring tasks assigned toKubernetesExecutor
not trigger Celery worker scaling.Testing Steps
KEDA
enabled and multiple executors:KubernetesExecutor
andqueue='default'
executor='KubernetesExecutor'
Additional Notes:
Related Issues:
Closes: #49001
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in airflow-core/newsfragments.