Use approximate queue length for azure queue scaler #4003

moria97 · 2022-12-12T12:51:06Z

Use the approximate queue length instead of visible messages, see #4002

Checklist

When introducing a new scaler, I agree with the scaling governance policy
I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated and is aligned with our changelog requirements
A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
A PR is opened to update the documentation on (repo) (if applicable)
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4002

Relates to #

Signed-off-by: Yue Fei <yufei@microsoft.com>

tomkerkhove · 2023-01-03T07:58:22Z

By removing the visible count you are not actually achieving what you are suggesting in #4002

moria97 · 2023-01-03T08:02:30Z

By removing the visible count you are not actually achieving what you are suggesting in #4002

Removing the visible count will return the approximate message count directly, which contains messages that are queued and being processed. Not sure if there is anything wrong? @tomkerkhove

v-shenoy

LGTM

v-shenoy · 2023-01-03T08:28:35Z

/run-e2e azure_queue
Update: You can check the progress here

tomkerkhove · 2023-01-03T08:57:28Z

I have misread the issue, no worries.

tomkerkhove · 2023-01-03T09:37:46Z

Can you add an entry in the changelog please?

I've pinged @ahmelsayed privately as well to see if we can remember why we added that.

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

tomkerkhove · 2023-01-04T07:17:32Z

@ahmelsayed pointed out that this was changed in #2613 by @RamCohen as an optimization.

I think the optimization is still OK to have and we can skip this PR but I don't have a strong opinion.

moria97 · 2023-01-04T07:59:18Z

@tomkerkhove This optimization looks make sense to me. But the issue is not related to the optimization, but the visible message count used to count queue length.

Let's say "queue length" parameter is 1, there are 20 messages and scaled out to 20 instances and pulled message from queue, the queue size returned is 0 （visible message account）, but the actual value should be 20 (approximate queue length).

zroubalik

@moria97 @tomkerkhove @v-shenoy so what is the status of this PR?

antonio-policastro · 2023-02-27T11:15:14Z

I'm shipping a custom version of this scaler in production since v2.6.1. My custom versions have the exact same code as in this pull request, and they have been working perfectly fine for the past year. ApproximateMessageCount is the way to go.
There are multiple issues about this problem(eg: #2385 #3555).

The optimization done with #2613 will be useless, because we should always call GetProperties to get the ApproximateMessageCount.

I hope this will be merged soon.
Thanks

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

moria97 · 2023-02-28T06:26:44Z

@tomkerkhove @zroubalik Can you take a look at this pr?

v-shenoy

I approved this earlier also. I think we can (should) merge this.

@tomkerkhove @JorTurFer

JorTurFer

LGTM

CHANGELOG.md

tomkerkhove · 2023-02-28T09:07:42Z

/run-e2e azure_queue

Update: You can check the progress here

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

JorTurFer · 2023-02-28T12:21:18Z

/run-e2e azure_queue*
Update: You can check the progress here

wallflower762 · 2023-03-14T04:13:46Z

Thanks team, this is great!

Use approximate queue length

d18575f

Signed-off-by: Yue Fei <yufei@microsoft.com>

moria97 force-pushed the personal/yufei/fi-queue-length branch from 8d0366e to d18575f Compare December 12, 2022 12:59

moria97 marked this pull request as ready for review December 13, 2022 04:15

moria97 requested a review from a team as a code owner December 13, 2022 04:15

moria97 changed the title ~~Use approximate queue length~~ Use approximate queue length for azure queue scaler Dec 13, 2022

zroubalik requested review from v-shenoy and tomkerkhove January 2, 2023 10:09

v-shenoy approved these changes Jan 3, 2023

View reviewed changes

Merge branch 'main' into personal/yufei/fi-queue-length

ea16344

Update CHANGELOG.md

c7e4641

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

zroubalik reviewed Jan 20, 2023

View reviewed changes

Merge branch 'main' into personal/yufei/fi-queue-length

747ed9f

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

v-shenoy approved these changes Feb 28, 2023

View reviewed changes

JorTurFer approved these changes Feb 28, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

moria97 added 2 commits February 28, 2023 20:07

Update CHANGELOG.md

7670a48

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

Update CHANGELOG.md

220d6f4

Signed-off-by: Yue Fei <59813791+moria97@users.noreply.github.com>

JorTurFer enabled auto-merge (squash) February 28, 2023 12:27

JorTurFer merged commit 0ba2313 into kedacore:main Feb 28, 2023

xoanmm pushed a commit to xoanmm/keda that referenced this pull request Mar 22, 2023

Use approximate queue length for azure queue scaler (kedacore#4003)

d51958e

jellevankerk mentioned this pull request Apr 3, 2023

Azure Storage Queue scaling up to invisible messages #4432

Closed

leodip mentioned this pull request Jun 9, 2024

Update GetAzureQueueLength in azure queue scaler, to support different queue length strategies. #5875

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use approximate queue length for azure queue scaler #4003

Use approximate queue length for azure queue scaler #4003

moria97 commented Dec 12, 2022 •

edited by JorTurFer

Loading

tomkerkhove commented Jan 3, 2023

moria97 commented Jan 3, 2023

v-shenoy left a comment

v-shenoy commented Jan 3, 2023 •

edited by github-actions bot

Loading

tomkerkhove commented Jan 3, 2023

tomkerkhove commented Jan 3, 2023

tomkerkhove commented Jan 4, 2023

moria97 commented Jan 4, 2023

zroubalik left a comment

antonio-policastro commented Feb 27, 2023 •

edited

Loading

moria97 commented Feb 28, 2023

v-shenoy left a comment •

edited

Loading

JorTurFer left a comment

tomkerkhove commented Feb 28, 2023 •

edited by github-actions bot

Loading

JorTurFer commented Feb 28, 2023 •

edited by github-actions bot

Loading

wallflower762 commented Mar 14, 2023

Use approximate queue length for azure queue scaler #4003

Use approximate queue length for azure queue scaler #4003

Conversation

moria97 commented Dec 12, 2022 • edited by JorTurFer Loading

Checklist

tomkerkhove commented Jan 3, 2023

moria97 commented Jan 3, 2023

v-shenoy left a comment

Choose a reason for hiding this comment

v-shenoy commented Jan 3, 2023 • edited by github-actions bot Loading

tomkerkhove commented Jan 3, 2023

tomkerkhove commented Jan 3, 2023

tomkerkhove commented Jan 4, 2023

moria97 commented Jan 4, 2023

zroubalik left a comment

Choose a reason for hiding this comment

antonio-policastro commented Feb 27, 2023 • edited Loading

moria97 commented Feb 28, 2023

v-shenoy left a comment • edited Loading

Choose a reason for hiding this comment

JorTurFer left a comment

Choose a reason for hiding this comment

tomkerkhove commented Feb 28, 2023 • edited by github-actions bot Loading

JorTurFer commented Feb 28, 2023 • edited by github-actions bot Loading

wallflower762 commented Mar 14, 2023

moria97 commented Dec 12, 2022 •

edited by JorTurFer

Loading

v-shenoy commented Jan 3, 2023 •

edited by github-actions bot

Loading

antonio-policastro commented Feb 27, 2023 •

edited

Loading

v-shenoy left a comment •

edited

Loading

tomkerkhove commented Feb 28, 2023 •

edited by github-actions bot

Loading

JorTurFer commented Feb 28, 2023 •

edited by github-actions bot

Loading