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

Improvements to EC2 metadata handling #995

Merged
merged 7 commits into from Apr 28, 2017

Conversation

Projects
None yet
3 participants
@tas50
Member

tas50 commented Apr 28, 2017

There's a few changes here so far.

Better logging when we reject a version that Amazon says they support
Drop our timeouts to a more reasonable time. These APIs are crazy fast there's no need to wait 30 seconds
Only setup the http client object once and use methods that allow connection reuse
Don't parse the best API version 3 times
Add all current EC2 metadata versions

tas50 added some commits Apr 27, 2017

Remove double split
Not sure what the goal was here. The sort always works. Just roll with it

Signed-off-by: Tim Smith <tsmith@chef.io>
Up our keepalive timeout and drop out read timeout
Signed-off-by: Tim Smith <tsmith@chef.io>
Add all the metadata versions Amazon lists right now
We were still missing a few

Signed-off-by: Tim Smith <tsmith@chef.io>
Avoid parsing the best api version 2 extra times
We fetched all available versions 3 times and then we sorted and found the best results 3 times. That seems a bit silly.

Signed-off-by: Tim Smith <tsmith@chef.io>
Fix specs
Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50

This comment has been minimized.

Show comment
Hide comment
@tas50

tas50 Apr 28, 2017

Member

@ViktorSchlaffer This should resolve #994 by only creating a single net/http object and using the appropriate methods that allow connection reuse within net/http. I'm not able to verify the change using AWS however since they only support http 1.0 w/o keepalives. Any chance you can pull this branch down somewhere and let me know if you see a reduction in connections?

Member

tas50 commented Apr 28, 2017

@ViktorSchlaffer This should resolve #994 by only creating a single net/http object and using the appropriate methods that allow connection reuse within net/http. I'm not able to verify the change using AWS however since they only support http 1.0 w/o keepalives. Any chance you can pull this branch down somewhere and let me know if you see a reduction in connections?

@tas50 tas50 requested a review from btm Apr 28, 2017

Add additional debug logging to the metadata mixin
Signed-off-by: Tim Smith <tsmith@chef.io>
@ViktorSchlaffer

This comment has been minimized.

Show comment
Hide comment
@ViktorSchlaffer

ViktorSchlaffer Apr 28, 2017

@tas50 I've verified you changes in our Openstack environment, and now the ec2 plugin uses only one tcp connection for the http requests. I have not been able to deploy this on all of our instances, as that decision is not in my hands, but from a functional point of view, this is what I was looking for, so I expect, that it will resolve the connection pool starvation issue.
Thanks for your help Tim!

ViktorSchlaffer commented Apr 28, 2017

@tas50 I've verified you changes in our Openstack environment, and now the ec2 plugin uses only one tcp connection for the http requests. I have not been able to deploy this on all of our instances, as that decision is not in my hands, but from a functional point of view, this is what I was looking for, so I expect, that it will resolve the connection pool starvation issue.
Thanks for your help Tim!

@tas50 tas50 merged commit 01cd437 into master Apr 28, 2017

3 checks passed

DCO This commit has a DCO Signed-off-by
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tas50 tas50 deleted the reuse_metadata_connections branch Apr 28, 2017

@chef chef locked and limited conversation to collaborators Nov 16, 2017

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