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.

Sign up for GitHub

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

Jump to bottom

feat: Provide support for scaling apps using AWS SQS Queue to include in-flight messages as well #3132

Merged
merged 9 commits into from
Jun 17, 2022
Merged

feat: Provide support for scaling apps using AWS SQS Queue to include in-flight messages as well #3132

merged 9 commits into from
Jun 17, 2022

Conversation

LiamStorkey
Copy link
Contributor

@LiamStorkey LiamStorkey commented Jun 7, 2022 •
edited by zroubalik
Loading

Signed-off-by: Liam Storkey liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal

  • Added in a config option scaleOnInFlight which changes the formula used for the sqs queue scaling metrics
  • default is set to true and the fomula is set to the as currently used.
  • added in 2 new test cases
For the purposes of scaling, the default formula for "actual messages" is equal to `ApproximateNumberOfMessages` + `ApproximateNumberOfMessagesNotVislble`, since `NotVisible` in SQS terms means the message is still in-flight/processing. If you wish to only scale on `ApproximateNumberOfMessages` set `scaleOnInFlight` to `false`.
- `scaleOnInFlight` - Bool flag to set the metrics to scale on. When set to `false` "actual messages" is equal to `ApproximateNumberOfMessages`. When set to `true` "actual messages" is equal to `ApproximateNumberOfMessages` + `ApproximateNumberOfMessagesNotVislble`, since `NotVisible` in SQS terms means the message is still in-flight/processing. (default: true)

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • 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) (PR)
  • Changelog has been updated and is aligned with our changelog requirements

Relates to kedacore/keda-docs#773
Fixes #3133

Liam Storkey added 3 commits June 7, 2022 12:50
added new config options for scale on in flight metrics + changelog
3264b9a 
Signed-off-by: Liam Storkey <liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal>
updated changelog with link to docs pr
1f28afa 
Signed-off-by: Liam Storkey <liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal>
fixed indentation on test case
a8085fe 
Signed-off-by: Liam Storkey <liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal>
@LiamStorkey LiamStorkey requested a review from a team as a code owner June 7, 2022 03:02
@LiamStorkey LiamStorkey mentioned this pull request Jun 7, 2022
Merged
1 task
Liam Storkey added 2 commits June 7, 2022 13:12
Changed test case to string
846094f 
Signed-off-by: Liam Storkey <liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal>
run pre commit work
ff5f199 
Signed-off-by: Liam Storkey <liam.storkey@ip-10-0-0-12.ap-southeast-2.compute.internal>
@tomkerkhove tomkerkhove changed the title New config for aws sqs queue scaler feat: Provide support for scaling apps using AWS SQS Queue to include in-flight messages as well Jun 7, 2022
Liam Storkey added 3 commits June 16, 2022 10:45
Added case for scale on in flight for sqs queue scaler equal values test
285b525 
Signed-off-by: Liam Storkey <liam.storkey@SYD-C02FN3JPMD6R.local>
Fixed random python syntaxt _._
7d66160 
Signed-off-by: Liam Storkey <liam.storkey@SYD-C02FN3JPMD6R.local>
Fixed random if else
dacc3cc 
Signed-off-by: Liam Storkey <liam.storkey@SYD-C02FN3JPMD6R.local>
@LiamStorkey
Copy link
Contributor Author

LiamStorkey commented Jun 16, 2022 •
edited
Loading

hey @tomkerkhove
I have gotten all the test to pass (I'm trying to run e2e sooo fingers cossed that runs but i think I the bot checks if im a "team-member" - see above comment with slash command)

It would be amazing if we could get some eyes on this 👍🏻
Thanks

@LiamStorkey
Copy link
Contributor Author

/run-e2e

tomkerkhove
tomkerkhove approved these changes Jun 16, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@zroubalik
Copy link
Member

zroubalik commented Jun 16, 2022 •
edited by github-actions bot
Loading

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

zroubalik
zroubalik approved these changes Jun 16, 2022
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Updated docs wording
32450a4 
Signed-off-by: Liam Storkey <liam.storkey@syd-c02fn3jpmd6r.lan>
@tomkerkhove
Copy link
Member

@JorTurFer Do you know what's up with this GH check?

@JorTurFer
Copy link
Member

JorTurFer commented Jun 17, 2022 •
edited
Loading

There is a new commit since e2e test was executed. Every commit enqueue the check again (like other CIs because there could be changes in the code that should be tested)
Just skip them

@tomkerkhove tomkerkhove merged commit 2b3fdc0 into kedacore:main Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorTurFer JorTurFer JorTurFer approved these changes

@tomkerkhove tomkerkhove tomkerkhove approved these changes

@zroubalik zroubalik zroubalik approved these changes

Assignees
No one assigned
Labels
skip-e2e
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide support for scaling apps using AWS SQS Queue to include in-flight messages as well
4 participants
@LiamStorkey @zroubalik @tomkerkhove @JorTurFer