Bug 894375
| Summary: | --tcp_nodelay broker option to have yes/no values | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise MRG | Reporter: | Pavel Moravec <pmoravec> | ||||
| Component: | qpid-cpp | Assignee: | Andrew Stitcher <astitcher> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Ernie <eallen> | ||||
| Severity: | low | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | 2.2 | CC: | eallen, iboverma, jross, lzhaldyb, mtoth | ||||
| Target Milestone: | 3.0 | Keywords: | EasyFix, Patch | ||||
| Target Release: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | qpid-cpp-0.22-8 | Doc Type: | Bug Fix | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | |||||||
| : | 961009 (view as bug list) | Environment: | |||||
| Last Closed: | 2014-09-24 15:05:08 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 768114, 961008, 961009 | ||||||
| Attachments: |
|
||||||
|
Description
Pavel Moravec
2013-01-11 15:33:55 UTC
Created attachment 676927 [details]
Proposed patch
I think a sensible pattern is to adopt -enabled as a suffix for long options that take booleans. This has the virtue of fixing the misfeature of program options and getopt that allows you to shorten long options, causing ambiguities (ugh!). But before we get there, I think I've been a little confused about this one. We can support a default no-arg behavior for --tcp-nodelay, true? That is: --tcp-nodelay no --> disabled --tcp-nodelay yes --> enabled --tcp-nodelay --> our code default (which would change to enabled) Justin (In reply to comment #4) > I think a sensible pattern is to adopt -enabled as a suffix for long options > that take booleans. This has the virtue of fixing the misfeature of program > options and getopt that allows you to shorten long options, causing > ambiguities (ugh!). > > But before we get there, I think I've been a little confused about this one. > We can support a default no-arg behavior for --tcp-nodelay, true? That is: > > --tcp-nodelay no --> disabled > --tcp-nodelay yes --> enabled > --tcp-nodelay --> our code default (which would change to enabled) > I think I have a workable variant of this: We introduce a new option --tcp-nodelay-enable (we are already using "-enable" as a suffix in a couple of places, but not using "-enabled"). so we have 3 possible options now: --tcp-nodelay-enable no --tcp-nodelay-enable yes --tcp-nodelay BUT the first 2 can be abbreviated to --tcp-nodelay no & --tcp-nodelay yes; So I think we have the result we are looking for. (In reply to comment #5) > (In reply to comment #4) > > I think a sensible pattern is to adopt -enabled as a suffix for long options > > that take booleans. This has the virtue of fixing the misfeature of program > > options and getopt that allows you to shorten long options, causing > > ambiguities (ugh!). > > > > But before we get there, I think I've been a little confused about this one. > > We can support a default no-arg behavior for --tcp-nodelay, true? That is: > > > > --tcp-nodelay no --> disabled > > --tcp-nodelay yes --> enabled > > --tcp-nodelay --> our code default (which would change to enabled) > > > > I think I have a workable variant of this: > > We introduce a new option --tcp-nodelay-enable (we are already using > "-enable" as a suffix in a couple of places, but not using "-enabled"). > > so we have 3 possible options now: > --tcp-nodelay-enable no > --tcp-nodelay-enable yes > --tcp-nodelay > > BUT the first 2 can be abbreviated to --tcp-nodelay no & --tcp-nodelay yes; > So I think we have the result we are looking for. Unfortunately the abbreviation doesn't work in the current code base. Moreso, the non command line configuration options - config file and environment variables work "correctly" as is and you are forced to use tcp-nodelay=yes|no in the config file and QPID_TCP_NODELAY=yes|no as an environment variable so they don't need a new name at all. The change to resolve this issue has now gone into the qpid code base on trunk for 0.23 in r1469088. The following should be the basis of documentation/release note. For RHEL 6 and later: Note that this introduces the possibility of command line options --tcp-nodelay=yes & --tcp-nodelay=no (and all the other ways of doing similar) For RHEL5: It was not possible to easily add optional arguments to command line options because it is not supported by the boost library used (1.33). As the default is now equivalent to always specifying --tcp-nodelay it is necessary to use the config file or an environment variable to turn the option off if that is desired on RHEL5. A necessary fix to the original change has gone in to r1469466 on qpid trunk Summary: Failed to disable to the TCP_NODELAY option using the config file. All other methods succeed (command line and environment variable) For rhel 6.4 32/64 Verified for the command line options --tcp-nodelay no --tcp-nodelay off --tcp-nodelay 0 --tcp-nodelay false --tcp-nodelay --tcp-nodelay yes --tcp-nodelay on --tcp-nodelay 1 --tcp-nodelay true If no --tcp-nodelay argument then TCP_NODELAY is enabled If --tcp-nodelay is set to junk (--tcp-nodelay=blah), then broker doesn't start. Verified for environment variable QPID_TCP_NODELAY=no QPID_TCP_NODELAY=off QPID_TCP_NODELAY=0 QPID_TCP_NODELAY=false QPID_TCP_NODELAY= QPID_TCP_NODELAY=yes QPID_TCP_NODELAY=on QPID_TCP_NODELAY=1 QPID_TCP_NODELAY=true If QPID_TCP_NODELAY is set to junk (blah), then broker doesn't start. Verified for config file No entry in config file results in TCP_NODELAY being enabled FAILED for config file tcp-nodelay=no tcp-nodelay=off tcp-nodelay=0 tcp-nodelay=junk Basically, the broker will start with a tcp-nodelay=<anything> in the config file, but TCP_NODELAY is always enabled. There appears to be no way to disable TCP_NODELAY using the config file. Based on comment 7, disabling this option using the config file should be supported. Package list from rhel 6.4 64 ------------------------------ cyrus-sasl-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-devel-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-gssapi-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-lib-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-md5-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-plain-2.1.23-13.el6_3.1.x86_64 python-qpid-0.22-4.el6.noarch python-qpid-qmf-0.22-7.el6.x86_64 python-saslwrapper-0.22-3.el6.x86_64 qpid-cpp-client-0.22-8.el6.x86_64 qpid-cpp-client-devel-0.22-8.el6.x86_64 qpid-cpp-client-devel-docs-0.22-8.el6.noarch qpid-cpp-client-rdma-0.22-8.el6.x86_64 qpid-cpp-client-ssl-0.22-8.el6.x86_64 qpid-cpp-debuginfo-0.22-8.el6.x86_64 qpid-cpp-server-0.22-8.el6.x86_64 qpid-cpp-server-devel-0.22-8.el6.x86_64 qpid-cpp-server-ha-0.22-8.el6.x86_64 qpid-cpp-server-rdma-0.22-8.el6.x86_64 qpid-cpp-server-ssl-0.22-8.el6.x86_64 qpid-cpp-server-store-0.22-8.el6.x86_64 qpid-cpp-server-xml-0.22-8.el6.x86_64 qpid-cpp-tar-0.22-8.el6.noarch qpid-java-client-0.22-5.el6.noarch qpid-java-common-0.22-5.el6.noarch qpid-java-example-0.22-5.el6.noarch qpid-proton-c-0.4-2.2.el6.x86_64 qpid-proton-c-devel-0.4-2.2.el6.x86_64 qpid-proton-debuginfo-0.4-2.2.el6.x86_64 qpid-qmf-0.22-7.el6.x86_64 qpid-qmf-debuginfo-0.22-7.el6.x86_64 qpid-qmf-devel-0.22-7.el6.x86_64 qpid-snmpd-1.0.0-12.el6.x86_64 qpid-snmpd-debuginfo-1.0.0-12.el6.x86_64 qpid-tests-0.22-4.el6.noarch qpid-tools-0.22-3.el6.noarch saslwrapper-0.22-3.el6.x86_64 saslwrapper-devel-0.22-3.el6.x86_64 rhel 5 was not tested. Are you sure that you were modifying the correct config file? the location has changed I believe it should now be in /etc/qpid/qpidd.conf. Yes :) I was starting the broker with a local config file (using the --config argument). But just to be sure it was actually reading the config file, I added a "string=foo" line to the config file and tried to start the broker. It failed with an error parsing the config file. On a related note, The broker was able to start when the config file contained tcp-nodelay=random-junk It started with TCP_NODELAY enabled, but I suspect that is only because it is now enabled by default. The command line and environment variable parser complains when it can't parse the value into a boolean, but the config file parser doesn't. Data point: Disabling tcp-nodelay works as expected using Fedora 19 (gcc 4.8.1 & boost 1.53) and trunk qpid. Verified Retested with latest packages. tcp-nodelay is working as expected from the command line, environment and config file. The environment that failed was the 0.22-8 packages. I re-installed these and saw the failure when using the config file. However, when tested against the 0.22-13 packages, setting tcp-nodelay with the config file now works. Passed with packages ------------------ cyrus-sasl-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-devel-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-gssapi-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-lib-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-md5-2.1.23-13.el6_3.1.x86_64 cyrus-sasl-plain-2.1.23-13.el6_3.1.x86_64 perl-qpid-0.22-5.el6.x86_64 python-qpid-0.22-4.el6.noarch python-qpid-qmf-0.22-10.el6.x86_64 python-saslwrapper-0.22-3.el6.x86_64 qpid-cpp-client-0.22-13.el6.x86_64 qpid-cpp-client-devel-0.22-13.el6.x86_64 qpid-cpp-client-devel-docs-0.22-13.el6.noarch qpid-cpp-client-rdma-0.22-13.el6.x86_64 qpid-cpp-client-ssl-0.22-13.el6.x86_64 qpid-cpp-debuginfo-0.22-13.el6.x86_64 qpid-cpp-server-0.22-13.el6.x86_64 qpid-cpp-server-devel-0.22-13.el6.x86_64 qpid-cpp-server-ha-0.22-13.el6.x86_64 qpid-cpp-server-rdma-0.22-13.el6.x86_64 qpid-cpp-server-ssl-0.22-13.el6.x86_64 qpid-cpp-server-store-0.22-13.el6.x86_64 qpid-cpp-server-xml-0.22-13.el6.x86_64 qpid-cpp-tar-0.22-13.el6.noarch qpid-java-client-0.22-5.el6.noarch qpid-java-common-0.22-5.el6.noarch qpid-java-example-0.22-5.el6.noarch qpid-proton-c-0.5-2.el6.x86_64 qpid-proton-c-devel-0.5-2.el6.x86_64 qpid-proton-debuginfo-0.5-2.el6.x86_64 qpid-qmf-0.22-10.el6.x86_64 qpid-qmf-debuginfo-0.22-10.el6.x86_64 qpid-qmf-devel-0.22-10.el6.x86_64 qpid-snmpd-1.0.0-12.el6.x86_64 qpid-snmpd-debuginfo-1.0.0-12.el6.x86_64 qpid-tests-0.22-4.el6.noarch qpid-tools-0.22-3.el6.noarch rh-qpid-cpp-tests-0.22-13.el6.x86_64 ruby-qpid-0.7.946106-2.el6.x86_64 saslwrapper-0.22-3.el6.x86_64 saslwrapper-devel-0.22-3.el6.x86_64 Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHEA-2014-1296.html |