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

Ensure TCP_NODELAY is disabled by fixing namespacing #187

Merged
merged 1 commit into from
Feb 14, 2014
Merged

Ensure TCP_NODELAY is disabled by fixing namespacing #187

merged 1 commit into from
Feb 14, 2014

Conversation

nelgau
Copy link
Contributor

@nelgau nelgau commented Feb 14, 2014

While using bunny-1.1.0, we discovered an unacceptable latency for publishing messages with confirmation. Between a Bunny client and broker, both running Ubuntu Precise, this latency converges to 40ms for the case of many small messages. After some head-scratching, a packet capture revealed the true cause:

screen shot 2014-02-09 at 10 39 26 pm

Although both frames are sent in a tight loop, the client machine waits for an ACK before sending the payload. This suggests that Bunny isn't disabling Nagle's algorithm as a read through implies it ought to. After some poking, I discovered the bug:

     ::Socket.constants.include?(:TCP_NODELAY)  # => true
Bunny::Socket.constants.include?(:TCP_NODELAY)  # => false

By monkey-patching the implementation, I was able to achieve the desired result:

statsd airbnb

michaelklishin added a commit that referenced this pull request Feb 14, 2014
Ensure TCP_NODELAY is disabled by fixing namespacing
@michaelklishin michaelklishin merged commit 7f57ef8 into ruby-amqp:master Feb 14, 2014
michaelklishin added a commit to michaelklishin/sneakers that referenced this pull request Feb 15, 2014
[Significantly lower latency](ruby-amqp/bunny#187) for workloads that involve frequently sent small messages.
michaelklishin pushed a commit to ruby-amqp/hutch that referenced this pull request Feb 15, 2014
[Lower latency](ruby-amqp/bunny#187), you see.
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.

2 participants