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

Updated "Enable SO_REUSEPORT socket option" #1344

Merged
merged 1 commit into from
Oct 15, 2016

Conversation

kirubakaran
Copy link
Contributor

I've made the tweaks suggested by @berkerpeksag here: #1131

cc @benoitc @smilliken @tilgovi @pypeng

@@ -38,6 +38,11 @@ def __getattr__(self, name):

def set_options(self, sock, bound=False):
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
try:
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
except (AttributeError, socket.error) as e:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather see a check like if hasattr(socket, 'SO_REUSEPORT'):. If kernel doesn't support SO_REUSEPORT option, there is no point to try and print a warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benoitc @berkerpeksag Please see the updated PR.

@wooparadog
Copy link
Contributor

Continuing my comment in #1131 (comment) (though nobody comments on mine 😢 )

Very coincidentally, we actually find a use case where SO_REUSEPORT could benefit our applications.

First, some background:
We are using gunicorn_thrift to run thrift services instead of wsgi applications. So naturally, connections to our server would exist for quite a long time(1~20min).

In the native mechanism of how gunicorn processes sockets, it's very likely many connections are established to a few workers(lwn has stated it's also one of the reasons google implemented SO_REUSEPORT). So we are using the following patch to use SO_REUSEPORT in gunicorn.

Note that when --reuseport is enabled, master process will no longer create listening sockets.

https://github.com/eleme/gunicorn/commit/0ac40b460149f757cbbc04288f4764c3e261a645

@benoitc
Copy link
Owner

benoitc commented Sep 23, 2016

@wooparadog In the native mechanism of how gunicorn processes sockets, it's very likely many connections are established to a few workers no because we are using SO_REUSEADDR on the same couple addr:port. The main differerence in using SO_REUSEPORT is to check that it is using exactly the same couple addr:port :
http://stackoverflow.com/questions/14388706/socket-options-so-reuseaddr-and-so-reuseport-how-do-they-differ-do-they-mean-t#14388707

To answer to @berkerpeksag i think SO_REUSEPORT is the behaviour we want to use when it is available on the platform.

@wooparadog "when --reuseport is enabled, master process will no longer create listening sockets. " what ? We want to have the master listening on the socket. Workers are only accepting.

@berkerpeksag
Copy link
Collaborator

To answer to @berkerpeksag i think SO_REUSEPORT is the behaviour we want to use when it is available on the platform.

So that means we don't need a new config and #1339 can be closed, right?

@benoitc
Copy link
Owner

benoitc commented Sep 23, 2016

yes sorry to not have been clear ...

On Fri, Sep 23, 2016 at 3:49 PM Berker Peksag notifications@github.com
wrote:

To answer to @berkerpeksag https://github.com/berkerpeksag i think
SO_REUSEPORT is the behaviour we want to use when it is available on the
platform.

So that means we don't need a new config and #1339
#1339 can be closed, right?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1344 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA4ogdhbt0TsklykNilFJwWpuLgfZbnks5qs9jbgaJpZM4JxwdU
.

@wooparadog
Copy link
Contributor

@benoitc

no because we are using SO_REUSEADDR on the same couple addr:port. The main differerence in using SO_REUSEPORT is to check that it is using exactly the same couple addr:port :

I might also have not been clear... the following is quoted from the article about Linux's reuseport in lwn


The second of the traditional approaches used by multithreaded servers operating on a single port is to have all of the threads (or processes) perform an accept() call on a single listening socket in a simple event loop of the form:

while (1) {
    new_fd = accept(...);
    process_connection(new_fd);
}

The problem with this technique, as Tom pointed out, is that when multiple threads are waiting in the accept() call, wake-ups are not fair, so that, under high load, incoming connections may be distributed across threads in a very unbalanced fashion. At Google, they have seen a factor-of-three difference between the thread accepting the most connections and the thread accepting the fewest connections; that sort of imbalance can lead to underutilization of CPU cores. By contrast, the SO_REUSEPORT implementation distributes connections evenly across all of the threads (or processes) that are blocked in accept() on the same port.


Because we are using gunicorn for thrift servers(long connection server). The problem with non-fair accepting is quite serious. However SO_REUSEPORT solves this problem, but we have to create sockets in workers, not We want to have the master listening on the socket. Workers are only accepting.

I'm just giving ideas about how we use gunicorn and SO_REUSEPORT. For web servers(short connection server), our method should also increase utilization of CPU cores, but it doesn't matter that much as it does to a thrift server.

@benoitc
Copy link
Owner

benoitc commented Oct 13, 2016

@wooparadog We are in a deep agreement on how useful this option can be :) Like I said, SO_REUSEPORT is what we want to use when it's available on the platform (it is not on all OS/platforms). What I meant is that we don't want an option. Either it's here and we use it, or it's not and we are back to the current design.

We might want an option to force the current behaviour (aka "--no-reuseport") but I'm not sure this option is really needed.

Anyway I would accept any patch that do first the check of the reuse_port and use it, and in a specific commit add this possible option to disable it. Thoughts?

@benoitc
Copy link
Owner

benoitc commented Oct 13, 2016

also we need a way to test that change :)

@berkerpeksag berkerpeksag merged commit bfc807a into benoitc:master Oct 15, 2016
@berkerpeksag
Copy link
Collaborator

Thanks!

mjjbell pushed a commit to mjjbell/gunicorn that referenced this pull request Mar 16, 2018
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.

5 participants