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/d08a047c Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/d08a047c Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/d08a047c Branch: refs/heads/feature/GEODE-1392 Commit: d08a047c231d7f2820b4c9d41789caf1b1167be2 Parents: 8beb8af Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Mon May 16 08:05:00 2016 -0700 Committer: Kirk Lund <kl...@apache.org> Committed: Wed May 18 10:04:24 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/d08a047c/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/d08a047c/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/d08a047c/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/d08a047c/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/d08a047c/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