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 default transport to close connections, set timeout at client #24

Merged
merged 1 commit into from
May 2, 2015

Conversation

trinchan
Copy link
Contributor

Using current master under go version go1.3.1 linux/amd64, we found that many connections to sendgrid were being orphaned in a CLOSE_WAIT state, leading to file descriptors that are never released. We traced the issue to the use of a custom Transport to implement dial timeouts not setting deadlines on other TCP operations (read, write, etc). This PR uses the Client.Timeout field to let the stdlib handle the timing out of connections, while using the DefaultTransport so that other transport defaults are set.

The issue presents itself as thousands of CLOSE_WAIT connections to sendgrid when checking lsof -l

emails TCP ip-10-171-52-54.ec2.internal:39976->184.173.190.226-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:59229->184.173.190.227-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:39978->184.173.190.226-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:40039->184.173.190.226-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:59292->184.173.190.227-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:40041->184.173.190.226-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:59294->184.173.190.227-static.reverse.softlayer.com:https (CLOSE_WAIT)
emails TCP ip-10-171-52-54.ec2.internal:40043->184.173.190.226-static.reverse.softlayer.com:https (CLOSE_WAIT)

With this patch, the connections are properly closed after the request is finished and no such entries can be found.

The Client.Timeout field was added in Go 1.3, so I think the travis build will fail for Go 1.1. Please advise if a Go 1.1 solution is necessary as well.

@deckarep
Copy link
Contributor

deckarep commented May 1, 2015

@eddiezane - this change looks solid. Let me know if you need me to help test it.

eddiezane added a commit that referenced this pull request May 2, 2015
Use default transport to close connections, set timeout at client
@eddiezane eddiezane merged commit 37fccec into sendgrid:master May 2, 2015
@eddiezane
Copy link
Contributor

Thanks for contributing to SendGrid Open Source! We think it's awesome when community members contribute to our projects and want to celebrate that.

The following link will ask you to authenticate with Github (so we can verify your identity). You'll then be asked for your shipping address so that we can send you a thanks for contributing.

Click Here to Continue »

Once again, thank you!

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