You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2016/05/16 15:08:30 UTC

incubator-geode git commit: GEODE-1393 locator returns incorrect server information when starting up

Repository: incubator-geode
Updated Branches:
  refs/heads/develop 92805bbb9 -> 6523c97c9


GEODE-1393 locator returns incorrect server information when starting up

When a locator auto-reconnects its ServerLocator needs to initialize its
ControllerAdvisor so that it has server information to give to clients.
The ServerLocator was creating a new ControllerAdvisor but didn't ask it
to perform a handshake to fill in its profiles.

ReconnectDUnitTest had an existing testReconnectWithQuorum test that
wasn't doing what it was supposed to.  I've removed the TODO from that
test and modified it to force-disconnect the tests Locator.  The
locator must restart its TcpServer component before it can start
a DistributedSystem, so this exercises the path in
InternalLocator.attemptReconnect() that boots the TcpServer prior to
connecting the DistributedSystem.  After the DistributedSystem
finishes reconnecting the ServerLocator's distribution advisor
should have been initialized by performing the handshake.


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

Branch: refs/heads/develop
Commit: 6523c97c92f607746d80b11c7cb5315b1137f5a2
Parents: 92805bb
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Mon May 16 08:05:00 2016 -0700
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Mon May 16 08:06:49 2016 -0700

----------------------------------------------------------------------
 .../distributed/internal/InternalLocator.java   |  1 +
 .../distributed/internal/LocatorStats.java      | 31 -----------
 .../distributed/internal/ServerLocator.java     | 44 +++++++--------
 .../gemfire/cache30/ReconnectDUnitTest.java     | 57 ++++++++++----------
 .../internal/ServerLocatorJUnitTest.java        |  0
 5 files changed, 53 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6523c97c/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
index 7effa3d..7ad57ad 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
@@ -1350,6 +1350,7 @@ public class InternalLocator extends Locator implements ConnectListener {
         return response;
       }
     }
+
     private JmxManagerLocatorResponse findJmxManager(JmxManagerLocatorRequest request) {
       JmxManagerLocatorResponse result = null;
       // NYI

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6523c97c/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java
old mode 100644
new mode 100755
index d42a2b4..1140b1f
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java
@@ -117,13 +117,6 @@ public class LocatorStats {
   }
   
   
-  /**
-   * Used by tests to create an instance given its already existings stats.
-   */
-  public LocatorStats(Statistics stats) {
-    this._stats = stats;
-  }
-
   public final void setServerCount(int sc) {
     if(this._stats==null) {
       this.endpoints_known.set(sc);
@@ -140,14 +133,6 @@ public class LocatorStats {
     }
   }
   
-  public final void incLocatorRequests() {
-    if(this._stats==null) {
-      this.requests_to_locator.incrementAndGet();
-    } else {
-      this._stats.incLong(_REQUESTS_TO_LOCATOR, 1);
-    }
-  }  
-  
   public final void endLocatorRequest(long startTime) {
     long took = DistributionStats.getStatTime()-startTime;
     if(this._stats==null) {
@@ -180,14 +165,6 @@ public class LocatorStats {
   
   
   
-  public final void incLocatorResponses() {
-    if(this._stats==null) {
-      this.responses_from_locator.incrementAndGet();
-    } else {
-      this._stats.incLong(_RESPONSES_FROM_LOCATOR, 1);
-    }
-  }  
-  
   public final void setLocatorRequests(long rl) {
     if(this._stats==null) {
       this.requests_to_locator.set(rl);
@@ -218,14 +195,6 @@ public class LocatorStats {
     } else {
       this._stats.incLong(_SERVER_LOAD_UPDATES, 1);
     }
-  }  
-  
-  public void setRequestInProgress(int threads) {
-    if(this._stats!=null) {
-      this._stats.setInt(_REQUESTS_IN_PROGRESS, threads);
-    } else {
-      requestsInProgress.set(threads);
-    }
   }
   
   public void incRequestInProgress(int threads) {

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6523c97c/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/ServerLocator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/ServerLocator.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/ServerLocator.java
old mode 100644
new mode 100755
index e535b97..b37a50b
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/ServerLocator.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/ServerLocator.java
@@ -24,12 +24,12 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import com.gemstone.gemfire.internal.DataSerializableFixedID;
 import org.apache.logging.log4j.Logger;
 
 import com.gemstone.gemfire.CancelCriterion;
@@ -42,7 +42,6 @@ import com.gemstone.gemfire.cache.client.internal.locator.GetAllServersRequest;
 import com.gemstone.gemfire.cache.client.internal.locator.GetAllServersResponse;
 import com.gemstone.gemfire.cache.client.internal.locator.LocatorListRequest;
 import com.gemstone.gemfire.cache.client.internal.locator.LocatorListResponse;
-import com.gemstone.gemfire.cache.client.internal.locator.LocatorStatusRequest;
 import com.gemstone.gemfire.cache.client.internal.locator.LocatorStatusResponse;
 import com.gemstone.gemfire.cache.client.internal.locator.QueueConnectionRequest;
 import com.gemstone.gemfire.cache.client.internal.locator.QueueConnectionResponse;
@@ -176,34 +175,34 @@ public class ServerLocator implements TcpHandler, DistributionAdvisee {
       logger.debug("ServerLocator: Received request {}", request);
     }
 
-    Object response;
+    if ( ! (request instanceof ServerLocationRequest) ) {
+      throw new InternalGemFireException("Expected ServerLocationRequest, got " + request.getClass());
+    }
 
-    if (request instanceof ServerLocationRequest) {
-      if (request instanceof LocatorStatusRequest) {
+    Object response;
+    int id = ((DataSerializableFixedID)request).getDSFID();
+    switch (id) {
+      case DataSerializableFixedID.LOCATOR_STATUS_REQUEST:
         response = new LocatorStatusResponse()
           .initialize(this.port, this.hostName, this.logFile, this.memberName);
-      }
-      else if (request instanceof LocatorListRequest) {
+        break;
+      case DataSerializableFixedID.LOCATOR_LIST_REQUEST:
         response = getLocatorListResponse((LocatorListRequest) request);
-      }
-      else if (request instanceof ClientReplacementRequest) {
+        break;
+      case DataSerializableFixedID.CLIENT_REPLACEMENT_REQUEST:
         response = pickReplacementServer((ClientReplacementRequest) request);
-      }
-      else if (request instanceof GetAllServersRequest) {
+        break;
+      case DataSerializableFixedID.GET_ALL_SERVERS_REQUEST:
         response = pickAllServers((GetAllServersRequest) request);
-      }
-      else if (request instanceof ClientConnectionRequest) {
+        break;
+      case DataSerializableFixedID.CLIENT_CONNECTION_REQUEST:
         response = pickServer((ClientConnectionRequest) request);
-      }
-      else if (request instanceof QueueConnectionRequest) {
+        break;
+      case DataSerializableFixedID.QUEUE_CONNECTION_REQUEST:
         response = pickQueueServers((QueueConnectionRequest) request);
-      }
-      else {
+        break;
+      default:
         throw new InternalGemFireException("Unknown ServerLocationRequest: " + request.getClass());
-      }
-    }
-    else {
-      throw new InternalGemFireException("Expected ServerLocationRequest, got " + request.getClass());
     }
 
     if(logger.isDebugEnabled()) {
@@ -290,6 +289,9 @@ public class ServerLocator implements TcpHandler, DistributionAdvisee {
       this.loadSnapshot = new LocatorLoadSnapshot();
       this.ds = (InternalDistributedSystem)ds;
       this.advisor = ControllerAdvisor.createControllerAdvisor(this); // escapes constructor but allows field to be final
+      if (ds.isConnected()) {
+        this.advisor.handshake();  // GEODE-1393: need to get server information during restart
+      }
     }
   }
   

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6523c97c/geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java
index ca2c17b..6c63def 100755
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java
@@ -27,6 +27,7 @@ import com.gemstone.gemfire.distributed.internal.DistributionConfig;
 import com.gemstone.gemfire.distributed.internal.InternalDistributedSystem;
 import com.gemstone.gemfire.distributed.internal.InternalDistributedSystem.ReconnectListener;
 import com.gemstone.gemfire.distributed.internal.InternalLocator;
+import com.gemstone.gemfire.distributed.internal.ServerLocator;
 import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember;
 import com.gemstone.gemfire.distributed.internal.membership.gms.MembershipManagerHelper;
 import com.gemstone.gemfire.distributed.internal.membership.gms.mgr.GMSMembershipManager;
@@ -78,6 +79,7 @@ public class ReconnectDUnitTest extends CacheTestCase
           locatorPort = locPort;
           Properties props = getDistributedSystemProperties();
           locator = Locator.startLocatorAndDS(locatorPort, new File(""), props);
+          ReconnectDUnitTest.savedSystem = InternalDistributedSystem.getConnectedInstance();
           IgnoredException.addIgnoredException("com.gemstone.gemfire.ForcedDisconnectException||Possible loss of quorum");
 //          MembershipManagerHelper.getMembershipManager(InternalDistributedSystem.getConnectedInstance()).setDebugJGroups(true);
         } catch (IOException e) {
@@ -163,10 +165,6 @@ public class ReconnectDUnitTest extends CacheTestCase
     return factory.create();
   }
 
-  /*
-  TODO this test is not actually using quorum checks.  To do that it needs to
-  have the locator disconnect & reconnect
-   */
 
   public void testReconnectWithQuorum() throws Exception {
     // quorum check fails, then succeeds
@@ -174,7 +172,7 @@ public class ReconnectDUnitTest extends CacheTestCase
     Host host = Host.getHost(0);
     VM vm0 = host.getVM(0);
     VM vm1 = host.getVM(1);
-    VM vm2 = host.getVM(2);
+    VM locatorVm = host.getVM(locatorVMNumber);
     
     final int locPort = locatorPort;
 
@@ -210,33 +208,23 @@ public class ReconnectDUnitTest extends CacheTestCase
       }
     };
     
-    System.out.println("creating caches in vm0, vm1 and vm2");
+    System.out.println("creating caches in vm0 and vm1");
     vm0.invoke(create);
     vm1.invoke(create);
-    vm2.invoke(create);
-    
+
     // view is [locator(3), vm0(15), vm1(10), vm2(10)]
     
-    /* now we want to cause vm0 and vm1 to force-disconnect.  This may cause the other
-     * non-locator member to also disconnect, depending on the timing
+    /* now we want to kick out the locator and observe that it reconnects
+     * using its rebooted location service
      */
-    System.out.println("disconnecting vm0");
-    forceDisconnect(vm0);
-    Wait.pause(10000);
-    System.out.println("disconnecting vm1");
-    forceDisconnect(vm1);
+    System.out.println("disconnecting locator");
+    forceDisconnect(locatorVm);
+    waitForReconnect(locatorVm);
+
+    // if the locator reconnected it did so with its own location
+    // service since it doesn't know about any other locators
+    ensureLocationServiceRunning(locatorVm);
 
-    /* now we wait for them to auto-reconnect */
-    try {
-      System.out.println("waiting for vm0 to reconnect");
-      waitForReconnect(vm0);
-      System.out.println("waiting for vm1 to reconnect");
-      waitForReconnect(vm1);
-      System.out.println("done reconnecting vm0 and vm1");
-    } catch (Exception e) {
-      ThreadUtils.dumpAllStacks();
-      throw e;
-    }
   }
   
   public void testReconnectOnForcedDisconnect() throws Exception  {
@@ -418,6 +406,19 @@ public class ReconnectDUnitTest extends CacheTestCase
       }
     });
   }
+
+  /** this will throw an exception if location services aren't running */
+  private void ensureLocationServiceRunning(VM vm) {
+    vm.invoke(new SerializableRunnable("ensureLocationServiceRunning") {
+      public void run() {
+        InternalLocator intloc = (InternalLocator)locator;
+        ServerLocator serverLocator = intloc.getServerLocatorAdvisee();
+        // the initialization flag in the locator's ControllerAdvisor will
+        // be set if a handshake has been performed
+        assertTrue(serverLocator.getDistributionAdvisor().isInitialized());
+      }
+    });
+  }
   
   private DistributedMember waitForReconnect(VM vm) {
     return (DistributedMember)vm.invoke(new SerializableCallable("wait for Reconnect and return ID") {
@@ -456,7 +457,7 @@ public class ReconnectDUnitTest extends CacheTestCase
     Host host = Host.getHost(0);
     VM vm0 = host.getVM(0);
     VM vm1 = host.getVM(1);
-    VM vm3 = host.getVM(3);
+    VM locatorVm = host.getVM(3);
     DistributedMember dm, newdm;
     
     final int locPort = locatorPort;
@@ -467,7 +468,7 @@ public class ReconnectDUnitTest extends CacheTestCase
     final String xmlFileLoc = (new File(".")).getAbsolutePath();
 
     //This locator was started in setUp.
-    File locatorViewLog = new File(vm3.getWorkingDirectory(), "locator"+locatorPort+"views.log");
+    File locatorViewLog = new File(locatorVm.getWorkingDirectory(), "locator"+locatorPort+"views.log");
     assertTrue("Expected to find " + locatorViewLog.getPath() + " file", locatorViewLog.exists());
     long logSize = locatorViewLog.length();
 

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6523c97c/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/ServerLocatorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/ServerLocatorJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/ServerLocatorJUnitTest.java
old mode 100644
new mode 100755