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

Set custom PollingDelay of 5 seconds for Azure VirtualMachinesClient #40066

Merged
merged 1 commit into from
Jan 23, 2017
Merged

Set custom PollingDelay of 5 seconds for Azure VirtualMachinesClient #40066

merged 1 commit into from
Jan 23, 2017

Conversation

codablock
Copy link
Contributor

The default polling delay of 1 minute results in very long delays when
an Azure Disk is attached to a node. It gets worse as go-autorest
doubles the default delay to 2 minutes.

Please see: #35180 (comment)

Only the PollingDelay for VirtualMachinesClient is modified here to
avoid too much pressure on Azure quotas.

Release Nodes:

Reduce time needed to attach Azure disks

@codablock
Set custom PollingDelay of 5 seconds for Azure VirtualMachinesClient
ff38ac2 
The default polling delay of 1 minute results in very long delays when
an Azure Disk is attached to a node. It gets worse as go-autorest
doubles the default delay to 2 minutes.

Please see: kubernetes#35180 (comment)

Only the PollingDelay for VirtualMachinesClient is modified here to
avoid too much pressure on Azure quotas.
@k8s-ci-robot
Copy link
Contributor

Hi @codablock. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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. I understand the commands that are listed here.

@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 18, 2017
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 18, 2017
@codablock
Copy link
Contributor Author

I was asked to add some more information about timing data.

Without this change, it takes at least 2 minutes for attachment and also for detachment. This means, if a volume needs to be reattached to another node, it takes at least 4 minutes to complete. With this PR applied, this is reduced to something like 40 seconds per operation, which is not optimal but still a lot better.

These timing are only for the attach/detach part of the Azure volumes. It does not take into account the time needed to format and mount the volumes. There are other issues and PRs working on these.

@colemickens
Copy link
Contributor

LGTM. Burns some quota, but only when attaching/detaching and the speedup seems worth it.

@codablock codablock mentioned this pull request Jan 18, 2017
Open
@colemickens colemickens mentioned this pull request Jan 18, 2017
Closed
@brendandburns
Copy link
Contributor

@k8s-bot ok to test

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2017
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 5356a16 into kubernetes:master Jan 23, 2017
@codablock
Copy link
Contributor Author

Thanks for the LGTM :)

I think this is a good and safe candidate for a 1.5.x cherry-pick. I'm still not sure how this is initiated. Is it enough to ping the saad-ali?

@colemickens
Copy link
Contributor

ping @saad-ali for a 1.5.x cherry-pick.

@codablock codablock mentioned this pull request Feb 3, 2017
Merged
@saad-ali saad-ali added this to the v1.5 milestone Feb 3, 2017
@saad-ali saad-ali added cherrypick-candidate cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Feb 3, 2017
k8s-github-robot pushed a commit that referenced this pull request Feb 4, 2017
Merge pull request #40925 from vwfs/automated-cherry-pick-of-#40066-o…
84425b3 
…rigin-release-1.5

Automatic merge from submit-queue

Automated cherry pick of #40066

Cherry pick of #40066 on release-1.5.

#40066: Set custom PollingDelay of 5 seconds for Azure
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

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

@brendandburns brendandburns

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
8 participants
@codablock @k8s-ci-robot @k8s-reviewable @colemickens @brendandburns @k8s-github-robot @k8s-cherrypick-bot @saad-ali