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.

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

Tune batcher EC2 Describe* delays #2029

Merged
merged 1 commit into from
May 2, 2024

Conversation

AndrewSirenko
Copy link
Contributor

@AndrewSirenko AndrewSirenko commented May 1, 2024

Is this a bug fix or adding new feature?
Improvement

What is this PR about? / Why do we need it?
Given that no throttling is observed for DescribeVolumes with 7k volume scalability tests, we reduced the current 1 second max batch delay to 500ms, and set that as the standard batcher delay.

This PR makes sure each RPC takes ~0.25s of batch latency per EC2 Describe call on average (and worst case extra delay of 0.5s).

This PR also makes sure each batcher will execute twice per second. In the rare case that each batcher is executing at once, the combined 12 requests per second is under the default EC2 Non-Mutating Action Bucket Refill Rate of 20.

What testing is done?
Scalability tests with batcher delays of 1, 0.5, 0.3, and 0.2 seconds for DescribeVolumes and DescribeInstances.

Final 5000 pod scalability test on default limits account.

API Count OK Client Error Throttled
CreateVolume 5676 5030 0 646
DeleteVolume 5563 4999 2 562
AttachVolume 5646 4989 0 657
DetachVolume 5747 4996 101 650
DescribeVolumes 4482 4482 0 0
DescribeInstances 3760 3760 0 0

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 1, 2024
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 1, 2024
Copy link

github-actions bot commented May 1, 2024

Code Coverage Diff

This PR does not change the code coverage

@AndrewSirenko AndrewSirenko force-pushed the tuneBatch2 branch 4 times, most recently from 1bc125b to 458701d Compare May 1, 2024 17:35
@AndrewSirenko AndrewSirenko changed the title Tune batcher EC2 Describe* delays [WIP] Tune batcher EC2 Describe* delays May 1, 2024
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 1, 2024
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 1, 2024
@AndrewSirenko AndrewSirenko changed the title [WIP] Tune batcher EC2 Describe* delays Tune batcher EC2 Describe* delays May 1, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 1, 2024
Copy link
Member

@torredil torredil left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 1, 2024
@ConnorJC3
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ConnorJC3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2024
@k8s-ci-robot k8s-ci-robot merged commit a8d4dae into kubernetes-sigs:master May 2, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants