This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-6035
in repository https://gitbox.apache.org/repos/asf/geode.git

commit ab26199095e7c61d13b79b1f2c26b584d2528127
Author: Bruce Schuchardt <bschucha...@pivotal.io>
AuthorDate: Wed Nov 21 13:24:46 2018 -0800

    GEODE-6035 Increase backlog for peer-to-peer connection formation
    
    I've made a sweep through the code to make TCP/IP backlog defaults
    consistent.  All of them now default to 1280 but all are limited
    by the operating system limit.  On Linux this is usually 128 and
    is configured with the somaxconn setting.
    
    The figure 1280 was recommended by Pivotal field engineers.
---
 .../distributed/internal/tcpserver/TcpServer.java  |  2 -
 .../apache/geode/internal/cache/properties.html    | 26 ++++++++++---
 .../internal/cache/tier/sockets/AcceptorImpl.java  |  2 +-
 .../org/apache/geode/internal/tcp/TCPConduit.java  | 44 ++++++++++++++--------
 4 files changed, 50 insertions(+), 24 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
index 761bb3b..3abcc5c 100755
--- 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
+++ 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
@@ -119,8 +119,6 @@ public class TcpServer {
   // no longer static so that tests can test this system property
   private final int READ_TIMEOUT =
       Integer.getInteger(DistributionConfig.GEMFIRE_PREFIX + 
"TcpServer.READ_TIMEOUT", 60 * 1000);
-  // This is for backwards compatibility. The p2p.backlog flag used to be the 
only way to configure
-  // the locator backlog.
   private static final int P2P_BACKLOG = Integer.getInteger("p2p.backlog", 
1000);
   private static final int BACKLOG =
       Integer.getInteger(DistributionConfig.GEMFIRE_PREFIX + 
"TcpServer.BACKLOG", P2P_BACKLOG);
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/properties.html 
b/geode-core/src/main/java/org/apache/geode/internal/cache/properties.html
index dea752a..9d6cb60 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/properties.html
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/properties.html
@@ -262,12 +262,12 @@ TBA
 <dd>
 <em>Public:</em> false
 <p>
-<em>Integer</em> (default is 1000)
+<em>Integer</em> (default is 1280)
 <p>
 See <code>org.apache.geode.internal.cache.tier.sockets.AcceptorImpl</code>
 constructor.
 <p>
-This is the TCP accept backlog for the acceptor thread's listening socket.
+This is the TCP/IP "accept" backlog for client/server communications.
 </dd>
 
 <!-- -------------------------------------------------------  -->
@@ -2357,6 +2357,20 @@ TBA
 </dd>
 
 <!-- -------------------------------------------------------  -->
+    <dt><strong>gemfire.TcpServer.BACKLOG</strong></dt>
+    <dd>
+        <em>Public:</em> false
+        <p>
+            <em>Integer</em> default is 1280 but is limited by the OS 
somaxconn setting
+        <p>
+        <pre>
+   This property establishes a Locator's TCP/IP "accept" backlog
+   for locator communications.
+</pre>
+        <p>
+            TBA
+    </dd>
+<!-- -------------------------------------------------------  -->
 <dt><strong>gemfire.validateMessageSize</strong></dt>
 <dd>
 <em>Public:</em> false
@@ -2612,17 +2626,17 @@ TBA
 </dd>
 
 <!-- -------------------------------------------------------  -->
-<dt><strong>p2p.backlog</strong></dt>
+<dt><strong>p2p. </strong></dt>
 <dd>
 <em>Public:</em> false
 <p>
-<em>Integer</em> (default is 50)
+<em>Integer</em> (default is 1280 but limited by OS somaxconn setting)
 <p>
 See <code>org.apache.geode.internal.tcp.TCPConduit#BACKLOG</code>.
 <p>
 <pre>
-  backlog is the "accept" backlog configuration parameter all
-  conduits server socket */
+  backlog is the TCP/IP "accept" backlog configuration parameter for cluster
+  communications
 </pre>
 <p>
 TBA
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index 158fc68..deae928 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -229,7 +229,7 @@ public class AcceptorImpl implements Acceptor, Runnable, 
CommBufferPool {
   /**
    * The default value of the {@link ServerSocket} {@link 
#BACKLOG_PROPERTY_NAME}system property
    */
-  private static final int DEFAULT_BACKLOG = 1000;
+  private static final int DEFAULT_BACKLOG = 1280;
 
   /**
    * The system property name for setting the {@link ServerSocket}backlog
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java 
b/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
index 0b3b438..0057847 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
@@ -89,7 +89,18 @@ public class TCPConduit implements Runnable {
   private static int LISTENER_CLOSE_TIMEOUT;
 
   /**
-   * backlog is the "accept" backlog configuration parameter all conduits 
server socket
+   * BACKLOG is the "accept" backlog configuration parameter for the conduits 
server socket.
+   * In most operating systems this is limited to 128 by default and you must 
change
+   * the OS setting somaxconn to go beyond that limit. Note that setting this 
too high
+   * can have ramifications when disconnecting from the distributed system if 
a thread
+   * is trying to connect to another member that is not accepting connections 
quickly
+   * enough. Setting it too low can also have adverse effects because backlog 
overflows
+   * aren't handled well by most tcp/ip implementations, causing connect 
timeouts instead
+   * of expected ServerRefusedConnection exceptions.
+   * <p>
+   * Normally the backlog isn't that important because if it's full of 
connection requests
+   * a SYN "cookie" mechanism is used to bypass the backlog queue. If this is 
turned off
+   * though connection requests are dropped when the queue is full.
    */
   private static int BACKLOG;
 
@@ -142,8 +153,8 @@ public class TCPConduit implements Runnable {
     // only use direct buffers if we are using nio
     useDirectBuffers = USE_NIO && !Boolean.getBoolean("p2p.nodirectBuffers");
     LISTENER_CLOSE_TIMEOUT = Integer.getInteger("p2p.listenerCloseTimeout", 
60000).intValue();
-    // fix for bug 37730
-    BACKLOG = Integer.getInteger("p2p.backlog", HANDSHAKE_POOL_SIZE + 
1).intValue();
+    // note: bug 37730 concerned this defaulting to 50
+    BACKLOG = Integer.getInteger("p2p.backlog", 1280).intValue();
   }
 
   ///////////////// permanent conduit state
@@ -412,23 +423,24 @@ public class TCPConduit implements Runnable {
    * this.bindAddress, which must be set before invoking this method.
    */
   private void createServerSocket() {
-    int p = this.port;
-    int b = BACKLOG;
+    int serverPort = this.port;
+    int connectionRequestBacklog = BACKLOG;
     InetAddress bindAddress = this.address;
 
     try {
       if (this.useNIO) {
-        if (p <= 0) {
+        if (serverPort <= 0) {
 
-          socket = socketCreator.createServerSocketUsingPortRange(bindAddress, 
b, isBindAddress,
+          socket = socketCreator.createServerSocketUsingPortRange(bindAddress,
+              connectionRequestBacklog, isBindAddress,
               this.useNIO, 0, tcpPortRange);
         } else {
           ServerSocketChannel channel = ServerSocketChannel.open();
           socket = channel.socket();
 
           InetSocketAddress inetSocketAddress =
-              new InetSocketAddress(isBindAddress ? bindAddress : null, p);
-          socket.bind(inetSocketAddress, b);
+              new InetSocketAddress(isBindAddress ? bindAddress : null, 
serverPort);
+          socket.bind(inetSocketAddress, connectionRequestBacklog);
         }
 
         if (useNIO) {
@@ -450,11 +462,13 @@ public class TCPConduit implements Runnable {
         channel = socket.getChannel();
       } else {
         try {
-          if (p <= 0) {
-            socket = 
socketCreator.createServerSocketUsingPortRange(bindAddress, b, isBindAddress,
+          if (serverPort <= 0) {
+            socket = 
socketCreator.createServerSocketUsingPortRange(bindAddress,
+                connectionRequestBacklog, isBindAddress,
                 this.useNIO, this.tcpBufferSize, tcpPortRange);
           } else {
-            socket = socketCreator.createServerSocket(p, b, isBindAddress ? 
bindAddress : null,
+            socket = socketCreator.createServerSocket(serverPort, 
connectionRequestBacklog,
+                isBindAddress ? bindAddress : null,
                 this.tcpBufferSize);
           }
           int newSize = socket.getReceiveBufferSize();
@@ -473,7 +487,7 @@ public class TCPConduit implements Runnable {
     } catch (IOException io) {
       throw new ConnectionException(
           String.format("While creating ServerSocket on port %s with address 
%s",
-              new Object[] {Integer.valueOf(p), bindAddress}),
+              new Object[] {Integer.valueOf(serverPort), bindAddress}),
           io);
     }
   }
@@ -652,8 +666,6 @@ public class TCPConduit implements Runnable {
                 ex);
             break;
           }
-          othersock.setSoTimeout(0);
-          socketCreator.handshakeIfSocketIsSSL(othersock, 
idleConnectionTimeout);
         }
         if (stopped) {
           try {
@@ -759,6 +771,8 @@ public class TCPConduit implements Runnable {
 
   protected void basicAcceptConnection(Socket othersock) {
     try {
+      othersock.setSoTimeout(0);
+      socketCreator.handshakeIfSocketIsSSL(othersock, idleConnectionTimeout);
       getConTable().acceptConnection(othersock, new PeerConnectionFactory());
     } catch (IOException io) {
       // exception is logged by the Connection

Reply via email to