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

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

commit de08ea1f4bf59ac495c130d247a0d3333f8da98e
Author: Bruce Schuchardt <[email protected]>
AuthorDate: Tue Mar 10 14:13:17 2020 -0700

    GEODE-7866: Clean up geode-tcp-server module and add missing javadocs
    
    Added package-info javadoc
    
    Renamed ServerSocketCreatorImpl to ClusterSocketCreatorImpl and renamed
    variables associated with these objects
    
    Removed TcpServerGossipVersionDUnitTest, which was only needed in old
    pre-Geode code when transitioning from GemFire 6x to GemFire 7x.
    
    Added javadocs to classes and public methods.
    
    Changed TcpServerProductVersionDUnitTest to use a TcpServer instead of a
    Locator.  Modified the test to wait for the TcpServer to shut down after
    it receives a ShutdownMessage.
---
 ...ketCreator.java => SCClusterSocketCreator.java} |   6 +-
 .../apache/geode/internal/net/SocketCreator.java   |   4 +-
 .../geode/test/dunit/internal/DUnitLauncher.java   |   2 +-
 .../org/apache/geode/test/version/TestVersion.java |  29 +++-
 .../tcpserver/TcpServerGossipVersionDUnitTest.java | 162 ---------------------
 .../internal/tcpserver/TcpServerJUnitTest.java     |  57 ++++----
 .../TcpServerProductVersionDUnitTest.java          |  27 +++-
 .../tcpserver/AdvancedSocketCreatorImpl.java       |   6 +-
 .../tcpserver/ClientSocketCreatorImpl.java         |   4 +
 ...atorImpl.java => ClusterSocketCreatorImpl.java} |   8 +-
 .../internal/tcpserver/ConnectionWatcher.java      |   5 +-
 .../internal/tcpserver/HostAndPort.java            |  20 ++-
 .../internal/tcpserver/InfoRequest.java            |   3 +-
 .../internal/tcpserver/InfoResponse.java           |   5 +-
 .../internal/tcpserver/ProtocolChecker.java        |   6 +
 .../internal/tcpserver/ShutdownRequest.java        |   3 +-
 .../internal/tcpserver/ShutdownResponse.java       |   4 +-
 .../distributed/internal/tcpserver/TcpClient.java  |  23 ++-
 .../distributed/internal/tcpserver/TcpHandler.java |   4 +
 .../distributed/internal/tcpserver/TcpServer.java  | 103 ++++++++-----
 .../internal/tcpserver/TcpSocketCreatorImpl.java   |   6 +-
 .../internal/tcpserver/VersionRequest.java         |   3 +-
 .../internal/tcpserver/VersionResponse.java        |   4 +-
 .../{ShutdownResponse.java => package-info.java}   |  23 +--
 24 files changed, 232 insertions(+), 285 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/net/SCServerSocketCreator.java
 
b/geode-core/src/main/java/org/apache/geode/internal/net/SCClusterSocketCreator.java
similarity index 95%
rename from 
geode-core/src/main/java/org/apache/geode/internal/net/SCServerSocketCreator.java
rename to 
geode-core/src/main/java/org/apache/geode/internal/net/SCClusterSocketCreator.java
index 572b330..866aa44 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/net/SCServerSocketCreator.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/net/SCClusterSocketCreator.java
@@ -25,14 +25,14 @@ import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLServerSocket;
 
 import org.apache.geode.GemFireConfigException;
-import org.apache.geode.distributed.internal.tcpserver.ServerSocketCreatorImpl;
+import 
org.apache.geode.distributed.internal.tcpserver.ClusterSocketCreatorImpl;
 import org.apache.geode.internal.admin.SSLConfig;
 import org.apache.geode.net.SSLParameterExtension;
 
-class SCServerSocketCreator extends ServerSocketCreatorImpl {
+class SCClusterSocketCreator extends ClusterSocketCreatorImpl {
   private final SocketCreator coreSocketCreator;
 
-  protected SCServerSocketCreator(SocketCreator socketCreator) {
+  protected SCClusterSocketCreator(SocketCreator socketCreator) {
     super(socketCreator);
     coreSocketCreator = socketCreator;
   }
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java 
b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
index 333c74f..60eb803 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
@@ -193,7 +193,7 @@ public class SocketCreator extends TcpSocketCreatorImpl {
   // -------------------------------------------------------------------------
 
   protected void initializeCreators() {
-    serverSocketCreator = new SCServerSocketCreator(this);
+    clusterSocketCreator = new SCClusterSocketCreator(this);
     clientSocketCreator = new SCClientSocketCreator(this);
     advancedSocketCreator = new SCAdvancedSocketCreator(this);
   }
@@ -666,7 +666,7 @@ public class SocketCreator extends TcpSocketCreatorImpl {
   public ServerSocket createServerSocket(int nport, int backlog, InetAddress 
bindAddr,
       List<GatewayTransportFilter> transportFilters, int socketBufferSize) 
throws IOException {
     if (transportFilters.isEmpty()) {
-      return ((SCServerSocketCreator) forCluster())
+      return ((SCClusterSocketCreator) forCluster())
           .createServerSocket(nport, backlog, bindAddr, socketBufferSize, 
useSSL());
     } else {
       printConfig();
diff --git 
a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/DUnitLauncher.java
 
b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/DUnitLauncher.java
index 946f693..e1b4f63 100644
--- 
a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/DUnitLauncher.java
+++ 
b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/DUnitLauncher.java
@@ -96,7 +96,7 @@ public class DUnitLauncher {
   /**
    * VM ID for the VM to use for the debugger.
    */
-  static final int DEBUGGING_VM_NUM = -1;
+  public static final int DEBUGGING_VM_NUM = -1;
 
   /**
    * VM ID for the VM to use for the locator.
diff --git 
a/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java 
b/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java
index 6f9b52b..f961f03 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java
@@ -17,10 +17,14 @@ package org.apache.geode.test.version;
 import java.io.Serializable;
 import java.util.Objects;
 
+import org.apache.geode.internal.serialization.Version;
+
 public class TestVersion implements Comparable, Serializable {
+  public static final TestVersion CURRENT_VERSION = new 
TestVersion(VersionManager.CURRENT_VERSION);
+
   private final int major;
   private final int minor;
-  private final int patch;
+  private final int release;
 
   public static TestVersion valueOf(final String versionString) {
     return new TestVersion(versionString);
@@ -36,7 +40,7 @@ public class TestVersion implements Comparable, Serializable {
     if (split[2].contains("-incubating")) {
       split[2] = split[2].substring(0, split[2].length() - 
"-incubating".length());
     }
-    patch = Integer.parseInt(split[2]);
+    release = Integer.parseInt(split[2]);
   }
 
   /**
@@ -47,9 +51,18 @@ public class TestVersion implements Comparable, Serializable 
{
     return new TestVersion(version1).compareTo(new TestVersion(version2));
   }
 
+  public boolean isSameAs(Version version) {
+    if (equals(CURRENT_VERSION) && 
version.equals(Version.getCurrentVersion())) {
+      return true;
+    }
+    return release == version.getRelease()
+        && minor == version.getMinorVersion()
+        && major == version.getMajorVersion();
+  }
+
   @Override
   public String toString() {
-    return "" + major + "." + minor + "." + patch;
+    return "" + major + "." + minor + "." + release;
   }
 
 
@@ -64,18 +77,18 @@ public class TestVersion implements Comparable, 
Serializable {
     TestVersion that = (TestVersion) o;
     return major == that.major &&
         minor == that.minor &&
-        patch == that.patch;
+        release == that.release;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(major, minor, patch);
+    return Objects.hash(major, minor, release);
   }
 
-  public TestVersion(int major, int minor, int patch) {
+  public TestVersion(int major, int minor, int release) {
     this.major = major;
     this.minor = minor;
-    this.patch = patch;
+    this.release = release;
   }
 
   @Override
@@ -92,7 +105,7 @@ public class TestVersion implements Comparable, Serializable 
{
     if (comparison != 0) {
       return comparison;
     }
-    return Integer.compare(patch, other.patch);
+    return Integer.compare(release, other.release);
   }
 
   public int compareTo(int major, int minor, int patch) {
diff --git 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerGossipVersionDUnitTest.java
 
b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerGossipVersionDUnitTest.java
deleted file mode 100644
index f321a9a..0000000
--- 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerGossipVersionDUnitTest.java
+++ /dev/null
@@ -1,162 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
- * agreements. See the NOTICE file distributed with this work for additional 
information regarding
- * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the 
License. You may obtain a
- * copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software 
distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
- * or implied. See the License for the specific language governing permissions 
and limitations under
- * the License.
- */
-package org.apache.geode.distributed.internal.tcpserver;
-
-import static 
org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
-import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
-import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.fail;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.Properties;
-
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-
-import org.apache.geode.distributed.Locator;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
-import 
org.apache.geode.distributed.internal.membership.gms.locator.FindCoordinatorRequest;
-import 
org.apache.geode.distributed.internal.membership.gms.locator.FindCoordinatorResponse;
-import org.apache.geode.internal.AvailablePortHelper;
-import org.apache.geode.internal.InternalDataSerializer;
-import org.apache.geode.internal.inet.LocalHostUtil;
-import org.apache.geode.internal.net.SocketCreatorFactory;
-import org.apache.geode.internal.security.SecurableCommunicationChannel;
-import org.apache.geode.internal.serialization.Version;
-import org.apache.geode.test.dunit.Invoke;
-import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
-import org.apache.geode.test.junit.categories.MembershipTest;
-import org.apache.geode.test.version.VersionManager;
-
-/**
- * This tests the rolling upgrade for locators with different GOSSIPVERSION.
- */
-@Category({MembershipTest.class})
-public class TcpServerGossipVersionDUnitTest extends JUnit4DistributedTestCase 
{
-
-  @Override
-  public final void postSetUp() throws Exception {
-    disconnectAllFromDS();
-    Invoke.invokeInEveryVM("Set TcpServer.isTesting true", () -> {
-      TcpServer.isTesting = true;
-    });
-  }
-
-  @Override
-  public final void preTearDown() throws Exception {
-    Invoke.invokeInEveryVM("Set TcpServer.isTesting true", () -> {
-      TcpServer.isTesting = false;
-    });
-  }
-
-  /**
-   * This test starts two locators with current GOSSIPVERSION and then shuts 
down one of them and
-   * restart it with new GOSSIPVERSION and verifies that it has recovered the 
system View. Then we
-   * upgrade next locator.
-   */
-  @Test
-  public void testGossipVersionBackwardCompatibility() {
-
-    final VM locator0 = VM.getVM(0);
-    final VM locator1 = VM.getVM(1);
-    final VM locatorRestart0 = VM.getVM(2);
-    final VM member = VM.getVM(3);
-
-    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
-
-    // Create properties for locator0
-    final int port0 = ports[0];
-    final File logFile0 = null;
-
-    // Create properties for locator1
-    final int port1 = ports[1];
-    final File logFile1 = null;
-
-    final String locators =
-        VM.getHostName() + "[" + port0 + "]," + VM.getHostName() + "[" + port1 
+ "]";
-
-    final Properties props = new Properties();
-    props.setProperty(LOCATORS, locators);
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-
-    locator0.invoke("Starting first locator on port " + port0, () -> {
-      try {
-        TcpServer.getGossipVersionMapForTestOnly().put(TcpServer.TESTVERSION - 
100,
-            VersionManager.getInstance().getCurrentVersionOrdinal());
-
-        Locator.startLocatorAndDS(port0, logFile0, props);
-      } catch (IOException e) {
-        fail("Locator1 start failed with Gossip Version: " + 
TcpServer.GOSSIPVERSION + "!", e);
-      }
-    });
-
-    // Start a new member to add it to discovery set of locator0.
-    member.invoke("Start a member", () -> {
-      disconnectFromDS();
-      TcpServer.getGossipVersionMapForTestOnly().put(TcpServer.TESTVERSION - 
100,
-          VersionManager.getInstance().getCurrentVersionOrdinal());
-      InternalDistributedSystem.connect(props);
-    });
-
-    // Start locator1 with props.
-    locator1.invoke("Starting second locator on port " + port1,
-        () -> restartLocator(port1, logFile1, props));
-
-    // Stop first locator currently running in locator0 VM.
-    locator0.invoke("Stopping first locator", () -> {
-      Locator.getLocator().stop();
-      disconnectFromDS();
-    });
-
-    // Restart first locator in new VM.
-    locatorRestart0.invoke(() -> restartLocator(port0, logFile0, props));
-  }
-
-  private void restartLocator(int port0, File logFile0, Properties props) {
-    try {
-      TcpServer.TESTVERSION -= 100;
-      TcpServer.OLDTESTVERSION -= 100;
-      TcpServer.getGossipVersionMapForTestOnly().put(TcpServer.TESTVERSION,
-          VersionManager.getInstance().getCurrentVersionOrdinal());
-      TcpServer.getGossipVersionMapForTestOnly().put(TcpServer.OLDTESTVERSION,
-          Version.GFE_57.ordinal());
-
-      Locator.startLocatorAndDS(port0, logFile0, props);
-
-      // Start a gossip client to connect to first locator "locator0".
-      FindCoordinatorRequest req = new FindCoordinatorRequest(
-          new InternalDistributedMember("localhost", 1234));
-      FindCoordinatorResponse response;
-
-      response = (FindCoordinatorResponse) new TcpClient(SocketCreatorFactory
-          .getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
-          InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
-          InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer())
-              .requestToServer(new 
HostAndPort(LocalHostUtil.getLocalHost().getHostName(), port0),
-                  req, 5000);
-      assertThat(response).isNotNull();
-
-    } catch (IllegalStateException e) {
-      fail("a Locator start failed with Gossip Version: " + 
TcpServer.GOSSIPVERSION + "!", e);
-    } catch (Exception e) {
-      fail("b Locator start failed with Gossip Version: " + 
TcpServer.GOSSIPVERSION + "!", e);
-    }
-  }
-}
diff --git 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
 
b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
index 991c10b..7252c53 100644
--- 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
+++ 
b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
@@ -49,19 +49,21 @@ import org.junit.experimental.categories.Category;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
-import org.apache.geode.DataSerializable;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
 import org.apache.geode.distributed.internal.DistributionStats;
 import org.apache.geode.distributed.internal.InfoRequestHandler;
-import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.PoolStatHelper;
 import org.apache.geode.distributed.internal.ProtocolCheckerImpl;
+import org.apache.geode.distributed.internal.membership.gms.Services;
 import org.apache.geode.internal.AvailablePort;
-import org.apache.geode.internal.InternalDataSerializer;
 import 
org.apache.geode.internal.cache.client.protocol.ClientProtocolServiceLoader;
 import org.apache.geode.internal.logging.CoreLoggingExecutors;
 import org.apache.geode.internal.net.SocketCreatorFactory;
-import org.apache.geode.internal.security.SecurableCommunicationChannel;
+import org.apache.geode.internal.serialization.BasicSerializable;
+import org.apache.geode.internal.serialization.DSFIDSerializer;
+import org.apache.geode.internal.serialization.DSFIDSerializerFactory;
+import org.apache.geode.internal.serialization.DeserializationContext;
+import org.apache.geode.internal.serialization.SerializationContext;
 import org.apache.geode.test.junit.categories.MembershipTest;
 import org.apache.geode.util.internal.GeodeGlossary;
 
@@ -69,8 +71,8 @@ import org.apache.geode.util.internal.GeodeGlossary;
 public class TcpServerJUnitTest {
 
   private static final int TIMEOUT = 60 * 1000;
-  private/* GemStoneAddition */ InetAddress localhost;
-  private/* GemStoneAddition */ int port;
+  private InetAddress localhost;
+  private int port;
   private SimpleStats stats;
   private TcpServer server;
 
@@ -90,16 +92,17 @@ public class TcpServerJUnitTest {
 
     stats = new SimpleStats();
 
+    DSFIDSerializer serializer = new DSFIDSerializerFactory().create();
+    Services.registerSerializables(serializer);
     server = new TcpServer(port, localhost, handler,
         "server thread", new ProtocolCheckerImpl(null, new 
ClientProtocolServiceLoader()),
         DistributionStats::getStatTime,
         () -> CoreLoggingExecutors.newThreadPoolWithSynchronousFeed("locator 
request thread ",
-            InternalLocator.MAX_POOL_SIZE, stats, 
InternalLocator.POOL_IDLE_TIMEOUT,
+            100, stats, TIMEOUT,
             new ThreadPoolExecutor.CallerRunsPolicy()),
-        SocketCreatorFactory
-            
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
-        InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
-        InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer(),
+        null,
+        serializer.getObjectSerializer(),
+        serializer.getObjectDeserializer(),
         GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.READ_TIMEOUT",
         GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.BACKLOG");
     server.start();
@@ -158,10 +161,11 @@ public class TcpServerJUnitTest {
   }
 
   private TcpClient createTcpClient() {
-    return new TcpClient(SocketCreatorFactory
-        .getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
-        InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
-        InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
+    DSFIDSerializer serializer = new DSFIDSerializerFactory().create();
+    Services.registerSerializables(serializer);
+    return new TcpClient(new TcpSocketCreatorImpl(),
+        serializer.getObjectSerializer(),
+        serializer.getObjectDeserializer());
   }
 
   @Test
@@ -177,7 +181,8 @@ public class TcpServerJUnitTest {
       public void run() {
         Boolean delay = Boolean.valueOf(true);
         try {
-          tcpClient.requestToServer(new 
HostAndPort(localhost.getHostAddress(), port), delay,
+          tcpClient.requestToServer(new 
HostAndPort(localhost.getHostAddress(), port),
+              new TestObject(1),
               TIMEOUT);
         } catch (IOException e) {
           e.printStackTrace();
@@ -192,7 +197,7 @@ public class TcpServerJUnitTest {
       Thread.sleep(500);
       assertFalse(done.get());
       tcpClient.requestToServer(new HostAndPort(localhost.getHostAddress(), 
port),
-          Boolean.valueOf(false), TIMEOUT);
+          new TestObject(0), TIMEOUT);
       assertFalse(done.get());
 
       latch.countDown();
@@ -255,28 +260,30 @@ public class TcpServerJUnitTest {
     assertFalse(server.isAlive());
   }
 
-  private static class TestObject implements DataSerializable {
+  private static class TestObject implements BasicSerializable {
     int id;
 
-    public TestObject() {
+    public TestObject() {}
 
+    public TestObject(int id) {
+      this.id = id;
     }
 
     @Override
-    public void fromData(DataInput in) throws IOException {
+    public void fromData(DataInput in, DeserializationContext context) throws 
IOException {
       id = in.readInt();
     }
 
     @Override
-    public void toData(DataOutput out) throws IOException {
+    public void toData(DataOutput out, SerializationContext context) throws 
IOException {
       out.writeInt(id);
     }
 
   }
 
-  private/* GemStoneAddition */ static class EchoHandler implements TcpHandler 
{
+  private static class EchoHandler implements TcpHandler {
 
-    protected/* GemStoneAddition */ boolean shutdown;
+    protected boolean shutdown;
 
 
     @Override
@@ -316,8 +323,8 @@ public class TcpServerJUnitTest {
 
     @Override
     public Object processRequest(Object request) throws IOException {
-      Boolean delay = (Boolean) request;
-      if (delay.booleanValue()) {
+      TestObject delay = (TestObject) request;
+      if (delay.id > 0) {
         try {
           latch.await(120 * 1000, TimeUnit.MILLISECONDS);
         } catch (InterruptedException e) {
diff --git 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java
 
b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java
index 9bc0f31..6035b17 100644
--- 
a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java
+++ 
b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java
@@ -41,16 +41,20 @@ import org.junit.runners.Parameterized;
 
 import org.apache.geode.distributed.Locator;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
+import org.apache.geode.distributed.internal.InternalLocator;
 import 
org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave;
 import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.InternalDataSerializer;
 import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.internal.net.SocketCreatorFactory;
 import org.apache.geode.internal.security.SecurableCommunicationChannel;
+import org.apache.geode.internal.serialization.Version;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
 import org.apache.geode.test.dunit.DistributedTestUtils;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.SerializableRunnableIF;
 import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.internal.DUnitLauncher;
 import org.apache.geode.test.dunit.rules.DistributedRule;
 import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
 import org.apache.geode.test.version.TestVersion;
@@ -83,9 +87,9 @@ public class TcpServerProductVersionDUnitTest implements 
Serializable {
     SocketCreatorFactory.close();
   }
 
+
   private static final TestVersion oldProductVersion = getOldProductVersion();
-  private static final TestVersion currentProductVersion =
-      TestVersion.valueOf(VersionManager.CURRENT_VERSION);
+  private static final TestVersion currentProductVersion = 
TestVersion.CURRENT_VERSION;
 
   @Parameterized.Parameters(name = "{0}")
   public static Collection<VersionConfiguration> data() {
@@ -113,11 +117,8 @@ public class TcpServerProductVersionDUnitTest implements 
Serializable {
   }
 
   private enum VersionConfiguration {
-
-    // OLD_OLD(oldProductVersion, oldProductVersion),
     OLD_CURRENT(oldProductVersion, currentProductVersion),
     CURRENT_OLD(currentProductVersion, oldProductVersion);
-    // CURRENT_CURRENT(currentProductVersion, currentProductVersion);
 
     final TestVersion clientProductVersion;
     final TestVersion locatorProductVersion;
@@ -138,8 +139,13 @@ public class TcpServerProductVersionDUnitTest implements 
Serializable {
 
   @Test
   public void testAllMessageTypes() {
-    VM clientVM = 
Host.getHost(0).getVM(versions.clientProductVersion.toString(), 0);
-    VM locatorVM = 
Host.getHost(0).getVM(versions.locatorProductVersion.toString(), 1);
+    int clientVMNumber = 
versions.clientProductVersion.isSameAs(Version.CURRENT)
+        ? DUnitLauncher.DEBUGGING_VM_NUM : 0;
+    int locatorVMNumber = 
versions.locatorProductVersion.isSameAs(Version.CURRENT)
+        ? DUnitLauncher.DEBUGGING_VM_NUM : 0;
+    VM clientVM = 
Host.getHost(0).getVM(versions.clientProductVersion.toString(), clientVMNumber);
+    VM locatorVM =
+        Host.getHost(0).getVM(versions.locatorProductVersion.toString(), 
locatorVMNumber);
     int locatorPort = createLocator(locatorVM, true);
 
     clientVM.invoke("issue version request",
@@ -151,6 +157,13 @@ public class TcpServerProductVersionDUnitTest implements 
Serializable {
     clientVM.invoke("issue shutdown request",
         createRequestResponseFunction(locatorPort, 
ShutdownRequest.class.getName(),
             ShutdownResponse.class.getName()));
+    locatorVM.invoke("wait for locator to stop", () -> {
+      Locator locator = Locator.getLocator();
+      if (locator != null) {
+        ((InternalLocator) locator).stop(false, false, false);
+        GeodeAwaitility.await().until(() -> ((InternalLocator) 
locator).isStopped());
+      }
+    });
   }
 
   private SerializableRunnableIF createRequestResponseFunction(
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/AdvancedSocketCreatorImpl.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/AdvancedSocketCreatorImpl.java
index 2d1b6da..2fcf48a 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/AdvancedSocketCreatorImpl.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/AdvancedSocketCreatorImpl.java
@@ -24,6 +24,10 @@ import java.util.concurrent.ThreadLocalRandom;
 
 import org.apache.geode.util.internal.GeodeGlossary;
 
+/**
+ * AdvancedSocketCreatorImpl is constructed and held by a TcpSocketCreator. It 
is
+ * accessed through the method {@link TcpSocketCreator#forAdvancedUse()}.
+ */
 public class AdvancedSocketCreatorImpl implements AdvancedSocketCreator {
 
   public static final boolean ENABLE_TCP_KEEP_ALIVE;
@@ -133,7 +137,7 @@ public class AdvancedSocketCreatorImpl implements 
AdvancedSocketCreator {
                 new InetSocketAddress(isBindAddress ? ba : null, localPort);
             socket.bind(address, backlog);
           } else {
-            socket = 
socketCreator.serverSocketCreator.createServerSocket(localPort,
+            socket = 
socketCreator.clusterSocketCreator.createServerSocket(localPort,
                 backlog, isBindAddress ? ba : null,
                 tcpBufferSize, sslConnection);
           }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClientSocketCreatorImpl.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClientSocketCreatorImpl.java
index 2479f82..959bd04 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClientSocketCreatorImpl.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClientSocketCreatorImpl.java
@@ -17,6 +17,10 @@ package org.apache.geode.distributed.internal.tcpserver;
 import java.io.IOException;
 import java.net.Socket;
 
+/**
+ * ClientSocketCreatorImpl is constructed and held by a TcpSocketCreator. It is
+ * accessed through the method {@link TcpSocketCreator#forClient()}.
+ */
 public class ClientSocketCreatorImpl implements ClientSocketCreator {
   protected final TcpSocketCreatorImpl socketCreator;
 
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ServerSocketCreatorImpl.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClusterSocketCreatorImpl.java
similarity index 91%
rename from 
geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ServerSocketCreatorImpl.java
rename to 
geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClusterSocketCreatorImpl.java
index ad637ad..0b277a3 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ServerSocketCreatorImpl.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ClusterSocketCreatorImpl.java
@@ -21,10 +21,14 @@ import java.net.InetSocketAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
 
-public class ServerSocketCreatorImpl implements ClusterSocketCreator {
+/**
+ * ClusterSocketCreatorImpl is constructed and held by a TcpSocketCreator. It 
is
+ * accessed through the method {@link TcpSocketCreator#forCluster()}.
+ */
+public class ClusterSocketCreatorImpl implements ClusterSocketCreator {
   private final TcpSocketCreatorImpl socketCreator;
 
-  protected ServerSocketCreatorImpl(TcpSocketCreatorImpl socketCreator) {
+  protected ClusterSocketCreatorImpl(TcpSocketCreatorImpl socketCreator) {
 
     this.socketCreator = socketCreator;
   }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ConnectionWatcher.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ConnectionWatcher.java
index a68da56..3d29cc8 100755
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ConnectionWatcher.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ConnectionWatcher.java
@@ -17,9 +17,10 @@ package org.apache.geode.distributed.internal.tcpserver;
 import java.net.Socket;
 
 /**
- * ConnectionWatcher is used to observe tcp/ip connection formation in 
SockCreator implementations.
- *
+ * ConnectionWatcher is used to observe tcp/ip connection formation in 
socket-creator
+ * implementations.
  *
+ * @see AdvancedSocketCreator#connect(HostAndPort, int, ConnectionWatcher, 
boolean, int, boolean)
  */
 public interface ConnectionWatcher {
   /**
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/HostAndPort.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/HostAndPort.java
index 860eae5..91c4391 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/HostAndPort.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/HostAndPort.java
@@ -30,10 +30,19 @@ import 
org.apache.geode.internal.serialization.StaticSerialization;
 import org.apache.geode.internal.serialization.Version;
 
 /**
- * This class is serializable for testing. A number of client/server and WAN 
tests
+ * HostAndPort is a holder of a host name/address and a port. It is the primary
+ * way to specify a connection endpoint in the socket-creator methods.
+ * <p>
+ * Note: This class is serializable for testing. A number of client/server and 
WAN tests
  * transmit PoolAttributes between unit test JVMs using RMI. PoolAttributes are
  * Externalizable for this purpose and use Geode serialization to transmit 
HostAndPort
  * objects along with other attributes.
+ *
+ * @see TcpSocketCreator
+ * @see ClusterSocketCreator
+ * @see ClientSocketCreator
+ * @see AdvancedSocketCreator
+ * @see TcpClient
  */
 public class HostAndPort implements DataSerializableFixedID {
 
@@ -56,10 +65,9 @@ public class HostAndPort implements DataSerializableFixedID {
   }
 
   /**
-   * If location is not litteral IP address a new resolved {@link 
InetSocketAddress} is returned.
-   *
-   * @return resolved {@link InetSocketAddress}, otherwise stored {@link 
InetSocketAddress} if
-   *         literal IP address is used.
+   * Returns an InetSocketAddress for this host and port. An attempt is made 
to resolve the
+   * host name but if resolution fails an unresolved InetSocketAddress is 
returned. This return
+   * value will not hold an InetAddress, so calling getAddress() on it will 
return null.
    */
   public InetSocketAddress getSocketInetAddress() {
     if (socketInetAddress.isUnresolved()) {
@@ -151,4 +159,6 @@ public class HostAndPort implements DataSerializableFixedID 
{
   public Version[] getSerializationVersions() {
     return new Version[0];
   }
+
+
 }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoRequest.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoRequest.java
index c656edf..65d2b3d 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoRequest.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoRequest.java
@@ -21,8 +21,7 @@ import 
org.apache.geode.internal.serialization.BasicSerializable;
 /**
  * A request to the TCP server to provide information about the server
  *
- * @since GemFire 5.7
- *
+ * @deprecated this was created for the deprecated Admin API
  */
 public class InfoRequest implements BasicSerializable {
 }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoResponse.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoResponse.java
index f54ae2a..7177b5c 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoResponse.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/InfoResponse.java
@@ -25,9 +25,10 @@ import 
org.apache.geode.internal.serialization.SerializationContext;
 import org.apache.geode.internal.serialization.StaticSerialization;
 
 /**
- * A response from the TCP server with information about the server
+ * A response to an InfoRequest message
  *
- * @since GemFire 5.7
+ * @see TcpClient#getInfo(HostAndPort)
+ * @deprecated this was created for the deprecated Admin API
  */
 public class InfoResponse implements BasicSerializable {
   private String[] info;
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ProtocolChecker.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ProtocolChecker.java
index 8b54fdd..98da65e 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ProtocolChecker.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ProtocolChecker.java
@@ -17,6 +17,12 @@ package org.apache.geode.distributed.internal.tcpserver;
 import java.io.DataInputStream;
 import java.net.Socket;
 
+/**
+ * ProtocolChecker checks the given byte to determine whether it is a valid 
communication
+ * mode. A ProtocolChecker may optionally handle all communication on the 
given socket
+ * and return a true value. Otherwise if the given byte is a valid 
communication mode the
+ * checker should return false.
+ */
 public interface ProtocolChecker {
   boolean checkProtocol(Socket socket, DataInputStream input,
       int firstByte) throws Exception;
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownRequest.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownRequest.java
index 301d04b..518d37b 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownRequest.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownRequest.java
@@ -21,7 +21,8 @@ import 
org.apache.geode.internal.serialization.BasicSerializable;
 /**
  * A request to the TCP server to shutdown
  *
- * @since GemFire 5.7
+ * @see TcpClient#requestToServer(HostAndPort, Object, int)
+ * @see ShutdownResponse
  */
 public class ShutdownRequest implements BasicSerializable {
 }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
index 0c6b29b..c59dbc7 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
@@ -19,8 +19,10 @@ package org.apache.geode.distributed.internal.tcpserver;
 import org.apache.geode.internal.serialization.BasicSerializable;
 
 /**
- * A response from the TCP server that it received the shutdown request
+ * A response from the TCP server that it received a ShutdownRequest
  *
+ * @see TcpClient#requestToServer(HostAndPort, Object, int)
+ * @see ShutdownRequest
  * @since GemFire 5.7
  */
 public class ShutdownResponse implements BasicSerializable {
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
index d1089d0..f57b584 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
@@ -21,9 +21,12 @@ import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.InetAddress;
 import java.net.Socket;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.function.LongSupplier;
+import java.util.function.Supplier;
 
 import javax.net.ssl.SSLException;
 
@@ -39,11 +42,11 @@ import 
org.apache.geode.internal.serialization.VersionedDataOutputStream;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 
 /**
- * <p>
  * Client for the TcpServer component of the Locator.
- * </p>
  *
- * @since GemFire 5.7
+ * @see TcpServer#TcpServer(int, InetAddress, TcpHandler, String, 
ProtocolChecker,
+ *      LongSupplier, Supplier, TcpSocketCreator, ObjectSerializer, 
ObjectDeserializer, String,
+ *      String)
  */
 public class TcpClient {
 
@@ -63,6 +66,8 @@ public class TcpClient {
    * Constructs a new TcpClient
    *
    * @param socketCreator the SocketCreator to use in communicating with the 
Locator
+   * @param objectSerializer serializer for messages sent to the TcpServer
+   * @param objectDeserializer deserializer for responses from the TcpServer
    */
   public TcpClient(TcpSocketCreator socketCreator, final ObjectSerializer 
objectSerializer,
       final ObjectDeserializer objectDeserializer) {
@@ -72,7 +77,7 @@ public class TcpClient {
   }
 
   /**
-   * Stops the Locator running on a given host and port
+   * Stops the TcpServer running on a given host and port
    */
   public void stop(HostAndPort addr) throws java.net.ConnectException {
     try {
@@ -92,6 +97,8 @@ public class TcpClient {
    * Contacts the Locator running on the given host, and port and gets 
information about it. Two
    * <code>String</code>s are returned: the first string is the working 
directory of the locator
    * and the second string is the product directory of the locator.
+   *
+   * @deprecated this was created for the deprecated Admin API
    */
   public String[] getInfo(HostAndPort addr) {
     try {
@@ -115,7 +122,8 @@ public class TcpClient {
    * @param addr The locator's address
    * @param request The request message
    * @param timeout Timeout for sending the message and receiving a reply
-   * @return the reply
+   * @return the reply. This may return a null
+   *         if we're unable to form a connection to the TcpServer before the 
given timeout elapses
    */
   public Object requestToServer(HostAndPort addr, Object request, int timeout)
       throws IOException, ClassNotFoundException {
@@ -130,7 +138,10 @@ public class TcpClient {
    * @param request The request message
    * @param timeout Timeout for sending the message and receiving a reply
    * @param replyExpected Whether to wait for a reply
-   * @return The reply, or null if no reply is expected
+   * @return The reply, or null if no reply is expected. This may also return 
a null
+   *         if we're unable to form a connection to the TcpServer before the 
given timeout elapses
+   * @throws ClassNotFoundException if the deserializer throws this exception
+   * @throws IOException if there is a problem interacting with the server
    */
   public Object requestToServer(HostAndPort addr, Object request, int timeout,
       boolean replyExpected) throws IOException, ClassNotFoundException {
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpHandler.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpHandler.java
index 5e9c592..8d7816c 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpHandler.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpHandler.java
@@ -16,6 +16,10 @@ package org.apache.geode.distributed.internal.tcpserver;
 
 import java.io.IOException;
 
+/**
+ * A TcpHandler is the handler of messages not directly processed by TcpServer 
itself.
+ * The messages handled by TcpServer are those in this package.
+ */
 public interface TcpHandler {
   /**
    * Process a request and return a response
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
index 7f2d828..7ed30b7 100755
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
@@ -48,34 +48,27 @@ import 
org.apache.geode.logging.internal.executors.LoggingThread;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 
 /**
- * TCP server which listens on a port and delegates requests to a request 
handler. The server uses
- * expects messages containing a global version number, followed by a 
DataSerializable object
+ * TcpServer listens on a port and delegates requests to one or more request 
handlers. Use
+ * TcpClient to send messages to a TcpServer. Messages should all implement 
DataSerializableFixedID.
  * <p>
- * This code was factored out of GossipServer.java to allow multiple handlers 
to share the same
- * gossip server port.
- *
- * @since GemFire 5.7
+ * TcpServer accepts a connection, reads one request, passes the request to a 
handler,
+ * sends the reply and then closes the connection.
+ * <p>
+ * TcpServer is used in the Geode Locator service.
  */
 public class TcpServer {
 
   /**
-   * The version of the tcp server protocol
-   * <p>
-   * This should be incremented if the gossip message structures change
-   * <p>
-   * 0 - special indicator of a non-gossip message from a client<br>
-   * 1000 - gemfire 5.5 - using java serialization<br>
-   * 1001 - 5.7 - using DataSerializable and supporting server locator 
messages.<br>
-   * 1002 - 7.1 - sending GemFire version along with GOSSIP_VERSION in each 
request.
-   * <p>
-   * with the addition of support for all old versions of clients you can no 
longer change this
-   * version number
+   * GOSSIPVERSION is a remnant of the pre-open-source TcpServer. It was used 
to designate
+   * the on-wire protocol for TcpServer communications prior to the 
introduction of Geode's
+   * Version class. It should not be changed and exists for 
backward-compatibility.
    */
   public static final int GOSSIPVERSION = 1002;
 
-  // Don't change it ever. We did NOT send GemFire version in a Gossip request 
till 1001 version.
-  // This GOSSIPVERSION is used in _getVersionForAddress request for getting 
GemFire version of a
-  // GossipServer.
+  /**
+   * Version 1001 was the on-wire protocol version prior to the introduction 
of the use of
+   * Geode's Version class to designate the on-wire protocol.
+   */
   public static final int OLDGOSSIPVERSION = 1001;
 
   @MutableForTesting("The map used here is mutable, because some tests modify 
it")
@@ -84,15 +77,6 @@ public class TcpServer {
   public static final int GOSSIP_BYTE = 0;
   private static final String P2P_BACKLOG_PROPERTY_NAME = "p2p.backlog";
 
-  // For test purpose only
-  @MutableForTesting
-  public static boolean isTesting = false;
-  // Non-final field for testing to avoid any security holes in system.
-  @MutableForTesting
-  public static int TESTVERSION = GOSSIPVERSION;
-  @MutableForTesting
-  public static int OLDTESTVERSION = OLDGOSSIPVERSION;
-
   public static final long SHUTDOWN_WAIT_TIME = 60 * 1000;
 
   private static final Logger logger = LogService.getLogger();
@@ -122,9 +106,8 @@ public class TcpServer {
 
 
   /*
-   * Initialize versions map. Warning: This map must be compatible with all 
GemFire versions being
-   * handled by this member "With different GOSSIPVERION". If GOSSIPVERIONS 
are same for then
-   * current GOSSIPVERSION should be used.
+   * Old on-wire protocol map. This should be removed in a release that breaks 
all backward
+   * compatibility since it has been replaced with Geode's Version class.
    */
   private static Map<Integer, Short> createGossipToVersionMap() {
     HashMap<Integer, Short> map = new HashMap<>();
@@ -133,6 +116,25 @@ public class TcpServer {
     return map;
   }
 
+  /**
+   * The constructor for TcpServer
+   *
+   * @param port The port to listen on
+   * @param bind_address The bind-address to use (may be null)
+   * @param handler The TcpHandler that will process messages
+   * @param threadName The name to use in the listening thread
+   * @param protocolChecker A cut point for inserting a message handler with 
different serialization
+   * @param nanoTimeSupplier A time supplier
+   * @param executorServiceSupplier A provider of the executor to be used by 
handlers
+   * @param socketCreator The socket-creator that TcpServer should use. If 
null a default socket
+   *        creator is constructed
+   * @param objectSerializer The serializer
+   * @param objectDeserializer The deserializer
+   * @param readTimeoutPropertyName A system property name used to look up 
read timeout millis
+   * @param backlogLimitPropertyName A system property name used to establish 
the server socket
+   *        backlog
+   * @see #start()
+   */
   public TcpServer(int port, InetAddress bind_address, TcpHandler handler,
       String threadName, ProtocolChecker protocolChecker,
       final LongSupplier nanoTimeSupplier,
@@ -161,6 +163,9 @@ public class TcpServer {
     backlogLimit = Integer.getInteger(backlogLimitPropertyName, p2pBacklog);
   }
 
+  /**
+   * This method is used during a Geode auto-reconnect to restart the 
server-socket thread
+   */
   public void restarting() throws IOException {
     this.shuttingDown = false;
     startServerThread();
@@ -172,6 +177,12 @@ public class TcpServer {
         + System.identityHashCode(this.serverThread) + ";alive=" + 
this.serverThread.isAlive());
   }
 
+  /**
+   * After constructing a TcpServer use this method to start its server-socket 
listening thread.
+   * A TcpServer should be stopped via a ShutdownRequest made through a 
TcpClient.
+   *
+   * @see TcpClient#stop(HostAndPort)
+   */
   public void start() throws IOException {
     this.shuttingDown = false;
     startServerThread();
@@ -207,26 +218,46 @@ public class TcpServer {
     }
   }
 
+  /**
+   * Wait on the server-socket thread using {@link Thread#join(long)}
+   *
+   * @param millis how long to wait
+   */
   public void join(long millis) throws InterruptedException {
     if (serverThread != null) {
       serverThread.join(millis);
     }
   }
 
+  /**
+   * Wait on the server-socket thread using {@link Thread#join()}
+   */
   public void join() throws InterruptedException {
     if (serverThread != null) {
       serverThread.join();
     }
   }
 
+  /**
+   * Check to see if the server-socket thread is alive
+   */
   public boolean isAlive() {
     return serverThread != null && serverThread.isAlive();
   }
 
+  /**
+   * Check to see if we've requested that the server-socket thread has been 
requested
+   * to shut down
+   */
   public boolean isShuttingDown() {
     return this.shuttingDown;
   }
 
+  /**
+   * Returns the server-socket's local socket address
+   *
+   * @see ServerSocket#getLocalSocketAddress()
+   */
   public SocketAddress getBindAddress() {
     return srv_sock.getLocalSocketAddress();
   }
@@ -462,15 +493,11 @@ public class TcpServer {
   }
 
   public static int getCurrentGossipVersion() {
-    return TcpServer.isTesting ? TcpServer.TESTVERSION : 
TcpServer.GOSSIPVERSION;
+    return GOSSIPVERSION;
   }
 
   public static int getOldGossipVersion() {
-    return TcpServer.isTesting ? TcpServer.OLDTESTVERSION : 
TcpServer.OLDGOSSIPVERSION;
-  }
-
-  public static Map<Integer, Short> getGossipVersionMapForTestOnly() {
-    return GOSSIP_TO_GEMFIRE_VERSION_MAP;
+    return OLDGOSSIPVERSION;
   }
 
 }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpSocketCreatorImpl.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpSocketCreatorImpl.java
index 8cb33e6..5849b08 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpSocketCreatorImpl.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpSocketCreatorImpl.java
@@ -26,7 +26,7 @@ import java.net.Socket;
 public class TcpSocketCreatorImpl implements TcpSocketCreator {
 
 
-  protected ServerSocketCreatorImpl serverSocketCreator;
+  protected ClusterSocketCreatorImpl clusterSocketCreator;
   protected ClientSocketCreatorImpl clientSocketCreator;
   protected AdvancedSocketCreatorImpl advancedSocketCreator;
 
@@ -35,7 +35,7 @@ public class TcpSocketCreatorImpl implements TcpSocketCreator 
{
   }
 
   protected void initializeCreators() {
-    serverSocketCreator = new ServerSocketCreatorImpl(this);
+    clusterSocketCreator = new ClusterSocketCreatorImpl(this);
     clientSocketCreator = new ClientSocketCreatorImpl(this);
     advancedSocketCreator = new AdvancedSocketCreatorImpl(this);
   }
@@ -55,7 +55,7 @@ public class TcpSocketCreatorImpl implements TcpSocketCreator 
{
 
   @Override
   public ClusterSocketCreator forCluster() {
-    return serverSocketCreator;
+    return clusterSocketCreator;
   }
 
   @Override
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionRequest.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionRequest.java
index 938e2f5..688fcfb 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionRequest.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionRequest.java
@@ -16,9 +16,10 @@ package org.apache.geode.distributed.internal.tcpserver;
 
 
 import org.apache.geode.internal.serialization.BasicSerializable;
+import org.apache.geode.internal.serialization.Version;
 
 /**
- * @since GemFire 7.1
+ * An internal message used by TcpClient to determine the {@linkplain Version} 
of a TcpServer
  */
 public class VersionRequest implements BasicSerializable {
 }
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionResponse.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionResponse.java
index 0117a76..2f86c0c 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionResponse.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/VersionResponse.java
@@ -24,9 +24,9 @@ import 
org.apache.geode.internal.serialization.SerializationContext;
 import org.apache.geode.internal.serialization.Version;
 
 /**
- * Get GemFire version of the member running TcpServer.
+ * An internal message sent back to TcpClient from a TcpServer to respond to a
  *
- * @since GemFire 7.1
+ * @{link VersionRequest}
  */
 public class VersionResponse implements BasicSerializable {
   private short versionOrdinal = Version.TOKEN.ordinal();
diff --git 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/package-info.java
similarity index 55%
copy from 
geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
copy to 
geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/package-info.java
index 0c6b29b..b3e6f27 100644
--- 
a/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/ShutdownResponse.java
+++ 
b/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/package-info.java
@@ -12,16 +12,17 @@
  * or implied. See the License for the specific language governing permissions 
and limitations under
  * the License.
  */
-
-package org.apache.geode.distributed.internal.tcpserver;
-
-
-import org.apache.geode.internal.serialization.BasicSerializable;
-
 /**
- * A response from the TCP server that it received the shutdown request
- *
- * @since GemFire 5.7
+ * The tcpserver package implements a request/response framework that is used 
by Geode's
+ * Locator service to process location requests. A TcpServer listens on a 
TCP/IP
+ * server socket and passes requests to the TcpHandler installed in the 
server. A TcpClient
+ * can be used to communicate with a TcpServer.
+ * <p>
+ * The tcpserver package also provides TcpSocketCreator and its dependent 
interfaces
+ * ClientSocketCreator, ClusterSocketCreator and AdvancedSocketCreator. Geode 
has versions
+ * of these in a higher-level package that support TLS as configured via 
Geode's "ssl"
+ * properties.
+ * <p>
+ * You can create a TcpSocketCreator with the implementation 
TcpSocketCreatorImpl.
  */
-public class ShutdownResponse implements BasicSerializable {
-}
+package org.apache.geode.distributed.internal.tcpserver;

Reply via email to