Bug 961008 - Docs: Enable TCP_NODELAY by default
Docs: Enable TCP_NODELAY by default
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: Messaging_Installation_and_Configuration_Guide (Show other bugs)
Development
Unspecified Unspecified
medium Severity low
: 3.0
: ---
Assigned To: Jared MORGAN
Eric Sammons
: Improvement
Depends On: 768114 894375 961009
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-08 10:05 EDT by Joshua Wulf
Modified: 2015-08-09 21:23 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: 768114
Environment:
Last Closed: 2015-01-22 10:28:31 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Apache JIRA QPID-3689 None None None Never

  None (edit)
Description Joshua Wulf 2013-05-08 10:05:49 EDT
+++ 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 05:14:15 EDT
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.