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

Allow modifyVolumeRequestHandlerTimeout to be configurable #1911

Closed
andrewcharlton opened this issue Jan 25, 2024 · 4 comments
Closed

Allow modifyVolumeRequestHandlerTimeout to be configurable #1911

andrewcharlton opened this issue Jan 25, 2024 · 4 comments

Comments

@andrewcharlton
Copy link
Contributor

Feature Request

Issue
When modifying our volumes, we change both the size and the iops/throughput annotations together so that we only perform a single volume modification call to AWS. However, we have seen a race condition where the iops/throughput gets modified by itself first, and then the volume is unable to resize because the volume is now in a 6 hour window.

Proposed Solution
There is a currently a hard coded 2 second window (the driver's modifyVolumeRequestHandlerTimeout) during which both size and iops/throughput modification requests must be received for them to be processed together. It is more important for us that they be processed together (and so avoid the 6 hour windows) than the speed at which they are processed, so we would like to extend this window.

I propose that this becomes an option in the ControllerOptions instead, defaulting to 2 seconds, so that users can tune this setting.

If this is reasonable, please let me know and I'll put a PR together. Thanks!
@adolsalamanca adolsalamanca mentioned this issue Jan 25, 2024
Merged
adolsalamanca added a commit to timescale/aws-ebs-csi-driver that referenced this issue Jan 26, 2024
@adolsalamanca
Make modify volume interval configurable (#8)
856b253 
We're having problems related to race conditions when several changes
are applied to volumes. We don't want those changes to be applied
independently, so we're extracting this param to a driverOption to
make it configurable from the outside.

Issue opened upstream:
kubernetes-sigs#1911
@ConnorJC3
Copy link
Contributor

Hi @andrewcharlton , we talked about this and you're good to go to submit a PR to make this change (note that you, or your employer if you're contributing on their behalf, will need to sign the standard Kubernetes CLA).

We'll get the PR reviewed as soon as time allows, thanks for contributing to the EBS CSI Driver!

@andrewcharlton andrewcharlton mentioned this issue Jan 30, 2024
Merged
@AndrewSirenko
Copy link
Contributor

/close

Due to fix being merged. Thanks for putting the PR together Andrew!

@k8s-ci-robot
Copy link
Contributor

@AndrewSirenko: Closing this issue.

In response to this:

/close

Due to fix being merged. Thanks for putting the PR together Andrew!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@andrewcharlton
Copy link
Contributor Author

Due to fix being merged. Thanks for putting the PR together Andrew!

Thank you all for the speedy reply/review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ConnorJC3 @andrewcharlton @k8s-ci-robot @AndrewSirenko