On Thu, 2019-01-17 at 21:46 +0100, Thibault Nélis wrote:
> Hello,
>
> The latest version in the 4.4 line (4.4.10) documents[0] the default
> value for the TCP_NODELAY socket option via SocketConfig.isTcpNoDelay
> as unset (false).
>
It is clearly a mistake, most likely just cut-n-paste.
I'll correct javadocs when I get around to it. You are welcome to raise
a PR at Github in order to speed things up.
Oleg
> org.apache.http.config.SocketConfig.isTcpNoDelay L128:
>
> /**
> * Determines the default value of the {@link
> java.net.SocketOptions#TCP_NODELAY} parameter
> * for newly created sockets.
> * <p>
> * Default: {@code false}
> * </p>
> *
> * @return the default value of the {@link
> java.net.SocketOptions#TCP_NODELAY} parameter.
> * @see java.net.SocketOptions#TCP_NODELAY
> */
> public boolean isTcpNoDelay() {
> return tcpNoDelay;
> }
>
> However, the builder sets it (sets its own tcpNoDelay field to true)
> by
> default before passing it in the SocketConfig constructor when
> building
> the object.
>
> org.apache.http.config.SocketConfig.Builder.Builder L233:
>
> Builder() {
> this.soLinger = -1;
> this.tcpNoDelay = true;
> }
>
> Given SocketConfig can only be instantiated with the builder and
> given
> the relevant field there isn't written to by anything (other than the
> public setTcpNoDelay method of course) I believe there is a mismatch
> between the documentation and reality.
>
> I'm not sure what the original intent is as both these lines were
> added
> in the same revision (1400016[1], git mirror commit ID
> 373066c3d7d5c6b4877cfbd09acca99dc0623650[2]) in October 2012. That
> being said, the same author previously (August 2012) explicitly set
> the
> default to true in HttpCore NIO in revision 1369809[3] (git mirror
> commit ID 3674f6d8e056df67dbf510857841acbae9aaf8be[4]) as part of
> HTTPCORE-306[5] released as part of version 4.3. In the previous
> version before the 4.3 line (in which the SocketConfig class was
> introduced) was branched off (4.2.1) it seems the default was to set
> it
> as well.
>
> [4.2.1] org.apache.http.params.HttpConnectionParams.getTcpNoDelay
> L100:
>
> /**
> * Obtains value of the {@link CoreConnectionPNames#TCP_NODELAY}
> parameter.
> * If not set, defaults to <code>true</code>.
> *
> * @param params HTTP parameters.
> * @return Nagle's algorithm flag
> */
> public static boolean getTcpNoDelay(final HttpParams params) {
> if (params == null) {
> throw new IllegalArgumentException("HTTP parameters may
> not
> be null");
> }
> return params.getBooleanParameter
> (CoreConnectionPNames.TCP_NODELAY, true);
> }
>
> I'm not sure how much research went into the decision; HTTPCORE-306
> at
> least doesn't seem to consider much of the nuances behind setting
> that
> option[6]. The earliest reference I could see in the codebase was in
> 2005 (set), prior to which it was probably using the operating system
> default (often unset). Given past and existing behavior in the
> codebase I think changing the documentation is the most sensible
> thing
> to do. FWIW I also think setting it is a reasonable default
> nowadays.
>
> Thanks for having a look into this, as a user the documentation was
> definitely confusing.
>
> 0:
>
http://hc.apache.org/httpcomponents-core-ga/httpcore/apidocs/org/apache/http/config/SocketConfig.html
> 1: https://svn.apache.org/viewvc?view=revision&revision=1400016
> 2:
>
https://github.com/apache/httpcomponents-core/commit/373066c3d7d5c6b4877cfbd09acca99dc0623650
> 3: https://svn.apache.org/viewvc?view=revision&revision=1369809
> 4:
>
https://github.com/apache/httpcomponents-core/commit/3674f6d8e056df67dbf510857841acbae9aaf8be
> 5: https://issues.apache.org/jira/browse/HTTPCORE-306
> 6: https://eklitzke.org/the-caveats-of-tcp-nodelay
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
