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 <[email protected]> Authored: Mon Aug 22 16:01:45 2016 -0700 Committer: Bruce Schuchardt <[email protected]> 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
