geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject incubator-geode git commit: Caching the CLUSTER component SocketCreator in TCPConduit
Date Mon, 22 Aug 2016 23:04:55 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-420 d3fbfbdf3 -> 075e10937


Caching the CLUSTER component SocketCreator in TCPConduit

This avoids fetching the SocketCreator each time it's going to be used.
TCPConduit holds onto it and it and Connection use the cached instance.

LocatorDUnitTest SSL tests were failing due to inadequate clean-up in
all of the DUnit JVMs.  Clean-up was only happening in the controller
JVM and in those that use the inherited distributed-system creation
methods.  LocatorDUnitTest can't use the inherited methods since they
force use of the DUnit Locator.  I've removed the FlakyTest designation
from the affected tests.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/075e1093
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/075e1093
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/075e1093

Branch: refs/heads/feature/GEODE-420
Commit: 075e109377274c0620b0ff06e43fd81a3cc2c1bb
Parents: d3fbfbd
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Mon Aug 22 16:01:45 2016 -0700
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Mon Aug 22 16:01:45 2016 -0700

----------------------------------------------------------------------
 .../gemfire/internal/tcp/Connection.java        |  7 ++-
 .../gemfire/internal/tcp/TCPConduit.java        | 52 ++++++++------------
 .../gemfire/distributed/LocatorDUnitTest.java   |  7 +--
 .../internal/JUnit4DistributedTestCase.java     |  2 +-
 4 files changed, 26 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/075e1093/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java
index 749e0cf..9ae0519 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java
@@ -33,9 +33,7 @@ import com.gemstone.gemfire.internal.logging.LogService;
 import com.gemstone.gemfire.internal.logging.LoggingThreadGroup;
 import com.gemstone.gemfire.internal.logging.log4j.AlertAppender;
 import com.gemstone.gemfire.internal.logging.log4j.LocalizedMessage;
-import com.gemstone.gemfire.internal.net.SSLEnabledComponent;
-import com.gemstone.gemfire.internal.net.SocketCreator;
-import com.gemstone.gemfire.internal.net.SocketCreatorFactory;
+import com.gemstone.gemfire.internal.net.*;
 import com.gemstone.gemfire.internal.tcp.MsgReader.Header;
 import com.gemstone.gemfire.internal.util.concurrent.ReentrantSemaphore;
 import org.apache.logging.log4j.Logger;
@@ -1289,7 +1287,8 @@ public class Connection implements Runnable {
         // socket = javax.net.ssl.SSLSocketFactory.getDefault()
         //  .createSocket(remoteAddr.getInetAddress(), remoteAddr.getPort());
         int socketBufferSize = sharedResource ? SMALL_BUFFER_SIZE : this.owner.getConduit().tcpBufferSize;
-        this.socket = SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER).connectForServer(
remoteAddr.getInetAddress(), remoteAddr.getDirectChannelPort(), socketBufferSize );
+        this.socket = owner.getConduit().getSocketCreator()
+                           .connectForServer( remoteAddr.getInetAddress(), remoteAddr.getDirectChannelPort(),
socketBufferSize );
         // Set the receive buffer size local fields. It has already been set in the socket.
         setSocketBufferSize(this.socket, false, socketBufferSize, true);
         setSendBufferSize(this.socket);

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/075e1093/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/TCPConduit.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/TCPConduit.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/TCPConduit.java
index 800a203..b8e067c 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/TCPConduit.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/TCPConduit.java
@@ -114,9 +114,11 @@ public class TCPConduit implements Runnable {
    */
   static boolean useDirectBuffers;
 
-  private volatile boolean inhibitNewConnections;
+  /**
+   * The socket producer used by the cluster
+   */
+  private final SocketCreator socketCreator;
 
-  //  private transient DistributedMembershipListener messageReceiver;
 
   private MembershipManager membershipManager;
 
@@ -280,6 +282,8 @@ public class TCPConduit implements Runnable {
         }
       }
     }
+    
+    this.socketCreator = SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER);
 
     startAcceptor();
   }
@@ -429,7 +433,7 @@ public class TCPConduit implements Runnable {
       if (this.useNIO) {
         if (p <= 0) {
 
-          socket = SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER).createServerSocketUsingPortRange(bindAddress,
b, isBindAddress, this.useNIO, 0, tcpPortRange);
+          socket = socketCreator.createServerSocketUsingPortRange(bindAddress, b, isBindAddress,
this.useNIO, 0, tcpPortRange);
         } else {
           ServerSocketChannel channel = ServerSocketChannel.open();
           socket = channel.socket();
@@ -459,10 +463,9 @@ public class TCPConduit implements Runnable {
       } else {
         try {
           if (p <= 0) {
-            socket = SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER)
-                                         .createServerSocketUsingPortRange(bindAddress, b,
isBindAddress, this.useNIO, this.tcpBufferSize, tcpPortRange);
+            socket = socketCreator.createServerSocketUsingPortRange(bindAddress, b, isBindAddress,
this.useNIO, this.tcpBufferSize, tcpPortRange);
           } else {
-            socket = SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER).createServerSocket(p,
b, isBindAddress ? bindAddress : null, this.tcpBufferSize);
+            socket = socketCreator.createServerSocket(p, b, isBindAddress ? bindAddress :
null, this.tcpBufferSize);
           }
           int newSize = socket.getReceiveBufferSize();
           if (newSize != this.tcpBufferSize) {
@@ -656,7 +659,7 @@ public class TCPConduit implements Runnable {
             logger.warn(LocalizedMessage.create(LocalizedStrings.TCPConduit_STOPPING_P2P_LISTENER_DUE_TO_SSL_CONFIGURATION_PROBLEM),
ex);
             break;
           }
-          SocketCreatorFactory.getSSLSocketCreatorForComponent(SSLEnabledComponent.CLUSTER).configureServerSSLSocket(othersock);
+          socketCreator.configureServerSSLSocket(othersock);
         }
         if (stopped) {
           try {
@@ -667,30 +670,9 @@ public class TCPConduit implements Runnable {
           }
           continue;
         }
-        if (inhibitNewConnections) {
-          //          if (logger.isTraceEnabled(LogMarker.QA)) {
-          logger.info("Test hook: inhibiting acceptance of connection {}", othersock);
-          //          }
-          othersock.close();
-          while (inhibitNewConnections && !stopped) {
-            this.stopper.checkCancelInProgress(null);
-            boolean interrupted = Thread.interrupted();
-            try {
-              Thread.sleep(2000);
-            } catch (InterruptedException e) {
-              interrupted = true;
-            } finally {
-              if (interrupted) {
-                Thread.currentThread().interrupt();
-              }
-            }
-          } // while
-          if (logger.isTraceEnabled(LogMarker.QA)) {
-            logger.trace(LogMarker.QA, "Test hook: finished inhibiting acceptance of connections");
-          }
-        } else {
-          acceptConnection(othersock);
-        }
+
+        acceptConnection(othersock);
+        
       } catch (ClosedByInterruptException cbie) {
         //safe to ignore
       } catch (ClosedChannelException e) {
@@ -1195,6 +1177,14 @@ public class TCPConduit implements Runnable {
   }
 
   /**
+   * returns the SocketCreator that should be used to produce
+   * sockets for TCPConduit connections.
+   * @return
+   */
+  protected SocketCreator getSocketCreator() {
+    return socketCreator;
+  }
+  /**
    * ARB: Called by Connection before handshake reply is sent.
    * Returns true if member is part of view, false if membership is not confirmed before
timeout.
    */

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/075e1093/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
index 851cff4..530cf20 100755
--- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
@@ -68,8 +68,7 @@ import com.gemstone.gemfire.test.dunit.VM;
 import com.gemstone.gemfire.test.dunit.Wait;
 import com.gemstone.gemfire.test.dunit.WaitCriterion;
 import com.gemstone.gemfire.test.dunit.internal.JUnit4DistributedTestCase;
-import com.gemstone.gemfire.test.junit.categories.DistributedTest;
-import com.gemstone.gemfire.test.junit.categories.FlakyTest;
+import com.gemstone.gemfire.test.junit.categories.*;
 import com.gemstone.gemfire.util.test.TestUtil;
 
 /**
@@ -130,7 +129,6 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase {
       system.disconnect();
       system = null;
     }
-    SocketCreatorFactory.close();
   }
 
   ////////  Test Methods
@@ -436,7 +434,6 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase {
   }
 
   @Test
-  @Category(FlakyTest.class)
   public void testStartTwoLocatorsOneWithSSLAndTheOtherNonSSL() throws Exception {
     IgnoredException expectedException = IgnoredException.addIgnoredException("Unrecognized
SSL message, plaintext connection");
     disconnectAllFromDS();
@@ -495,7 +492,6 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase {
   }
 
   @Test
-  @Category(FlakyTest.class)
   public void testStartTwoLocatorsOneWithNonSSLAndTheOtherSSL() throws Exception {
     IgnoredException expectedException = IgnoredException.addIgnoredException("Remote host
closed connection during handshake");
 
@@ -551,7 +547,6 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase {
   }
 
   @Test
-  @Category(FlakyTest.class)
   public void testStartTwoLocatorsWithDifferentSSLCertificates() throws Exception {
     IgnoredException expectedException = IgnoredException.addIgnoredException("Remote host
closed connection during handshake");
     IgnoredException expectedException2 = IgnoredException.addIgnoredException("unable to
find valid certification path to requested target");

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/075e1093/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
index cf3c240..6be3889 100755
--- a/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
@@ -165,7 +165,6 @@ public abstract class JUnit4DistributedTestCase implements DistributedTestFixtur
     }
     if (system == null || !system.isConnected()) {
       // Figure out our distributed system properties
-      SocketCreatorFactory.close();
       Properties p = DistributedTestUtils.getAllDistributedSystemProperties(props);
       lastSystemCreatedInTest = getTestClass(); // used to be getDeclaringClass()
       if (logPerTest) {
@@ -567,6 +566,7 @@ public abstract class JUnit4DistributedTestCase implements DistributedTestFixtur
     RegionTestCase.preSnapshotRegion = null;
     SocketCreator.resetHostNameCache();
     SocketCreator.resolve_dns = true;
+    SocketCreatorFactory.close();
     Message.MAX_MESSAGE_SIZE = Message.DEFAULT_MAX_MESSAGE_SIZE;
 
     // clear system properties -- keep alphabetized


Mime
View raw message