Bug 961008 - Docs: Enable TCP_NODELAY by default
Summary: Docs: Enable TCP_NODELAY by default
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: Messaging_Installation_and_Configuration_Guide
Version: Development
Hardware: Unspecified
OS: Unspecified
medium
low
Target Milestone: 3.0
: ---
Assignee: Jared MORGAN
QA Contact: Eric Sammons
URL:
Whiteboard:
Depends On: 768114 894375 961009
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-08 14:05 UTC by Joshua Wulf
Modified: 2015-08-10 01:23 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of: 768114
Environment:
Last Closed: 2015-01-22 15:28:31 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Apache JIRA QPID-3689 0 None None None Never

Description Joshua Wulf 2013-05-08 14:05:49 UTC
+++ This bug was initially created as a clone of Bug #768114 +++

See https://issues.apache.org/jira/browse/QPID-3689

--- Additional comment from Pavel Moravec on 2013-01-09 07:31:36 EST ---

When implementing this improvement, please make --tcp-nodelay option similar to --auth (i.e. one can specify if the option is "on" or "off"). Current behavior is quite misleading:

$ qpidd --trace --tcp-nodelay=no
2013-01-09 12:24:37 [System] debug Exception constructed: Error in command line options: extra parameter in 'tcp-nodelay'

$ qpidd --trace --tcp-nodelay no
..
(tcp_nodelay is switched _ON_)

$ qpidd --trace --tcp-nodelay false
..
(tcp_nodelay is switched _ON_)

$ qpidd --trace --tcp-nodelay=false
2013-01-09 12:29:37 [System] debug Exception constructed: Error in command line options: extra parameter in 'tcp-nodelay'

$ qpidd --trace
..
(tcp_nodelay is switched off)


Isn't it enough to apply this "patch" for this?

--- ./src/qpid/broker/Broker.cpp_orig	2013-01-09 12:31:22.599030546 +0100
+++ ./src/qpid/broker/Broker.cpp_new	2013-01-09 12:31:44.028904499 +0100
@@ -158,7 +158,7 @@ Broker::Options::Options(const std::stri
         ("auth", optValue(auth, "yes|no"), "Enable authentication, if disabled all incoming connections will be trusted")
         ("realm", optValue(realm, "REALM"), "Use the given realm when performing authentication")
         ("default-queue-limit", optValue(queueLimit, "BYTES"), "Default maximum size for queues (in bytes)")
-        ("tcp-nodelay", optValue(tcpNoDelay), "Set TCP_NODELAY on TCP connections")
+        ("tcp-nodelay", optValue(tcpNoDelay, "yes|no"), "Set TCP_NODELAY on TCP connections")
         ("require-encryption", optValue(requireEncrypted), "Only accept connections that are encrypted")
         ("known-hosts-url", optValue(knownHosts, "URL or 'none'"), "URL to send as 'known-hosts' to clients ('none' implies empty list)")
         ("sasl-config", optValue(saslConfigPath, "DIR"), "gets sasl config info from nonstandard location")

--- Additional comment from Andrew Stitcher on 2013-01-10 10:21:46 EST ---

(In reply to comment #1)
> ...
> Isn't it enough to apply this "patch" for this?
> ...
> -        ("tcp-nodelay", optValue(tcpNoDelay), "Set TCP_NODELAY on TCP
> connections")
> +        ("tcp-nodelay", optValue(tcpNoDelay, "yes|no"), "Set TCP_NODELAY on
> TCP connections")

What you changed in there is only the documentation string that appears in the usage message - sorry, if only it was that simple.

I think this request would better be tracked as a separate feature request as it is related but separate, Ie it may well be a good idea with a different priority from the original.

I say this because I think this is a usability request whereas the original is a performance related request.

However once the default is changed we will need a way to turn tcp nodelay off so this is probably a prerequisite.

--- Additional comment from Pavel Moravec on 2013-01-11 10:37:49 EST ---

(In reply to comment #2)
> (In reply to comment #1)
> > ...
> > Isn't it enough to apply this "patch" for this?
> > ...
> > -        ("tcp-nodelay", optValue(tcpNoDelay), "Set TCP_NODELAY on TCP
> > connections")
> > +        ("tcp-nodelay", optValue(tcpNoDelay, "yes|no"), "Set TCP_NODELAY on
> > TCP connections")
> 
> What you changed in there is only the documentation string that appears in
> the usage message - sorry, if only it was that simple.
--auth option is managed the same, I already tested it

> 
> I think this request would better be tracked as a separate feature request
> as it is related but separate, Ie it may well be a good idea with a
> different priority from the original.
See bz894375.

--- Additional comment from Andrew Stitcher on 2013-04-17 18:51:36 EDT ---

This is now fixed on the trunk of qpid (for 0.23) in r1469088.

Comment 2 Leonid Zhaldybin 2013-08-20 09:14:15 UTC
The documentation contains the description of --tcp-nodelay option now, which states that "This is set to on by default". This seems sufficient to me, verifying.


Note You need to log in before you can comment on or make changes to this bug.