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

Use custom http.Client for AWS #537

Merged
merged 1 commit into from
Aug 9, 2017
Merged

Conversation

aaron7
Copy link
Contributor

@aaron7 aaron7 commented Aug 9, 2017

The retry maxRetries change 0502e44 alerted to us with a problem with RequestErrors when talking to DynamoDB. Some queries are now reaching the maxRetry limit of 20 and fail.

After investigation https://github.com/weaveworks/service-conf/issues/1066 we believe these are caused by the sheer number of requests we are making due to non-persistent connections.

There is a bug in golang where the maximum idle connections per a host does not work as expected and golang kills idle connections aggressively golang/go#13801

This PR sets MaxIdleConnsPerHost explicitly to 100.


This change has been tested locally where we have seen Request Errors reduced from a constant stream to none.

Fixes https://github.com/weaveworks/service-conf/issues/1066

@aaron7 aaron7 requested a review from jml August 9, 2017 15:22
Copy link
Contributor

@jml jml left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@aaron7 aaron7 merged commit 7ab7a24 into master Aug 9, 2017
@aaron7 aaron7 deleted the increase-ideal-connections branch August 9, 2017 15:45
@bboreham
Copy link
Contributor

bboreham commented Apr 3, 2018

I suspect the underlying issue here was golang/go#22724, fixed in Go 1.10.1, but it was valid to make the problem go away by raising the number of open connections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants