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

Enable SO_REUSEPORT socket option. #1131

Closed
wants to merge 1 commit into from

Conversation

smilliken
Copy link

Needs:

  • Testing on Python <2.7.9 and 3.*
  • Configuration option.
  • Testing on non-linux systems?

try:
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
except socket.error as ex:
if ex.errno == 92:
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 use errno.ENOPROTOOPT instead of 92.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, perhaps we should raise the exception if exc.errno is not 92.

Copy link
Owner

Choose a reason for hiding this comment

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

@berkerpeksag in which case it isn't?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if it returns other than ENOPROTOOPT (e.g. EBADF, EINVAL), we could catch them in line 40 so ignore my last comment :)

@benoitc
Copy link
Owner

benoitc commented Dec 24, 2015

@smilliken interested by making these changes?

@tilgovi
Copy link
Collaborator

tilgovi commented Feb 15, 2016

There's been some silence here from @smilliken. If we're happy with this, we should just make the small changes and merge a copy of it, with attribution.

@berkerpeksag
Copy link
Collaborator

+1

@tilgovi tilgovi self-assigned this Mar 14, 2016
@pypeng
Copy link

pypeng commented Mar 28, 2016

+1

@benoitc
Copy link
Owner

benoitc commented Jul 27, 2016

which changes are needed? If someone can update the PR so we can merge it it would be helpful :)

kirubakaran added a commit to kirubakaran/gunicorn that referenced this pull request Aug 31, 2016
kirubakaran added a commit to kirubakaran/gunicorn that referenced this pull request Aug 31, 2016
@wooparadog
Copy link
Contributor

I have some doubt about adding reuseport support for gunicorn.

SO_REUSEPORT is primarily used for using the same listening port for two unrelated processes. But in gunicorn's case, all workers are forked from master, AFTER master opening the listening port, so all workers are already using the same port.

The only use case I can think of is (even more) graceful restart(starting another gunicorn process tree, using SO_REUSEPORT, then kill the old one), but gunicorn already has support for reloading worker processes...

But of course, adding SO_REUSEPORT won't hurt..

@berkerpeksag
Copy link
Collaborator

Superseded by #1344.

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.

6 participants