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

Support passing TCP_NODELAY to underlying socket #253

Closed
honzakral opened this issue Oct 4, 2013 · 3 comments
Closed

Support passing TCP_NODELAY to underlying socket #253

honzakral opened this issue Oct 4, 2013 · 3 comments

Comments

@honzakral
Copy link

Many people (including me) use urllib3 (or requests) to consume an API or to write a client for potential high-volume connections. In many cases performance of this can be severly impacted by the Nagle's Algorithm. A solution to this is to run .setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) on the underlying socket. This is however very hard to do with urllib3. The code to do so:

import socket
from urllib3.connectionpool import HTTPConnectionPool, HTTPConnection,\
    HTTPSConnectionPool, log as urllib3_log

class NoNagleHTTPConnection(HTTPConnection):
    def connect(self):
        super(NoNagleHTTPConnection, self).connect()
        self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)

class NoNagleHTTPConnectionPool(HTTPConnectionPool):
    def _new_conn(self):    
        """    
        Return a fresh :class:`httplib.HTTPConnection`.
        """    
        self.num_connections += 1      
        urllib3_log.info("Starting new HTTP connection (%d): %s" %
                (self.num_connections, self.host))
        return NoNagleHTTPConnection(host=self.host,
                            port=self.port,
                            strict=self.strict)

... and this is just when you use HTTPConnectionPool directly and for http only (more difficult for HTTPS).

I'd like to propose a solution to this problem, either being able to pass in a connection_class to be able to inject your own class (NoNagleHTTPConnection in my case). This could still get a little tricky with http/https.

Another possible solution is to be able to provide a callback that will be passed to the underlying connection class and called after the socket has been created.

Third solution that comes to mind is to simply support a flag (or just always specify TCP_NODELAY) to turn this behavior on.

I am leaning towards third but want to check with people before I do anything - I am more than happy to work on the solution since I use urllib3 as the default transport for the new Elasticsearch client - needless to say that performance is crucial for us there.

@shazow
Copy link
Member

shazow commented Oct 4, 2013

+1 for supporting TCP_NODELAY, but I'm reluctant to hardcode it into all new connections.

Let me mess around with making the first option feasible—I believe it's good design to do this anyways. We can talk about the other options after. :) Will ping this when I have something.

@kevinburke
Copy link
Contributor

This can be closed now

@shazow
Copy link
Member

shazow commented Jan 27, 2014

Yay!

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

No branches or pull requests

3 participants