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

[@aws-cdk/aws-autoscaling] The property targetRequestsPerSecond on autoScalingGroup.scaleOnRequestCount is by minute not second #11446

Closed
nibty opened this issue Nov 12, 2020 · 1 comment · Fixed by #11457
Closed

[@aws-cdk/aws-autoscaling] The property targetRequestsPerSecond on autoScalingGroup.scaleOnRequestCount is by minute not second #11446

nibty opened this issue Nov 12, 2020 · 1 comment · Fixed by #11457
Assignees
@NetaNir
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@nibty
Copy link

nibty commented Nov 12, 2020

Scaling by request count on an AutoSclaing group is done by minute not by second. The documentation and the property name suggests the scaling target is per second.

Reproduction Steps

asg = autoscaling.AutoScalingGroup(...)

asg.scale_on_request_count(
    "RpsTarget",
     target_requests_per_second=30,
)

What did you expect to happen?

I expected the auto scaling group to scale to the target of 30 requests per second.

What actually happened?

The auto scaling group scaled to the target of 30 requests per minute.

Environment

  • CDK 1.73.0 (build eb6f3a9)
  • 1.73.0
  • Node v12.18.3
  • Mac OS 11.0.1
  • Python 3.8.5

This is 🐛 Bug Report
@nibty nibty added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 12, 2020
@github-actions github-actions bot added the @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling label Nov 12, 2020
rix0rrr added a commit that referenced this issue Nov 13, 2020
@rix0rrr
fix(autoscaling): targetRequestsPerSecond is actually requests per …
63b818f 
…minute

Scaling to a request rate per instance was advertised to scale to a
rate/second, but the underlying API was actually expecting a
rate/minute.

Upscale the value of `targetRequestsPerSecond` by a factor of 60 to
make its name actually correct, deprecate it, and add a
`targetRequestsPerMinute` property which should have been the original
API.

Fixes #11446.
@rix0rrr rix0rrr mentioned this issue Nov 13, 2020
Merged
@mergify mergify bot closed this as completed in #11457 Nov 13, 2020
mergify bot pushed a commit that referenced this issue Nov 13, 2020
@rix0rrr
fix(autoscaling): targetRequestsPerSecond is actually requests per …
39e277f 
…minute (#11457)

Scaling to a request rate per instance was advertised to scale to a
rate/second, but the underlying API was actually expecting a
rate/minute.

Upscale the value of `targetRequestsPerSecond` by a factor of 60 to
make its name actually correct, deprecate it, and add a
`targetRequestsPerMinute` property which should have been the original
API.

Fixes #11446.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NetaNir NetaNir

Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(autoscaling): targetRequestsPerSecond is actually requests per minute aws/aws-cdk
2 participants
@nibty @NetaNir