Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

Refactor: +SocketMixin #114

Closed
wants to merge 7 commits into from
Closed

Refactor: +SocketMixin #114

wants to merge 7 commits into from

Conversation

digitalextremist
Copy link
Member

Right now, socket optimizations happen per-connection vs. per-server, and SSLServer is not covered... which this refactor resolves by using SocketMixin across both servers, and going full per-server vs. per-connection.

@digitalextremist
Copy link
Member Author

This might conflict some with #113. Not sure what order wouldn't produce a conflict.

end

def deoptimize_socket(socket)
socket.setsockopt(6, 3, 0) if socket.kind_of? TCPSocket
Copy link
Contributor

Choose a reason for hiding this comment

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

These magic constants could use a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

RUBY_PLATFORM only, or all the Socket:: constants and the symbols also?

The Socket constants are notoriously hard to find documentation on, but RUBY_PLATFORM will be set to whatever the OS is, and only Linux supports the behavior expected of #optimize_socket

@digitalextremist
Copy link
Member Author

This seems like a good time to also add a Keep-Alive timeout option, rather than having a hard-coded default, which also I cannot seem to find. Seems like there might not even be a default, and I see threads staying open purely for Keep-Alive now that pipelining is supported. Do you know where I can add this @tarcieri or @halorgium?

… more deoptimizations, and non-linux optimizations.
@tarcieri
Copy link
Member

tarcieri commented Nov 9, 2013

@digitalextremist the timeout situation is a bit dire. I'm in the process of refactoring all the requests and response state machines into Celluloid::FSMs

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

Successfully merging this pull request may close these issues.

3 participants