You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by hi...@apache.org on 2016/04/22 19:37:53 UTC
[1/2] incubator-geode git commit: GEODE-1150 removed unused code in
attemptToJoin function
Repository: incubator-geode
Updated Branches:
refs/heads/develop 70c5467e2 -> 582b53716
GEODE-1150 removed unused code in attemptToJoin function
1.Removed unused code in attemptToJoin function
2.Now we don't send removal messageto member if we see join
request from it.
3. Fixed test issue for above changes.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/582b5371
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/582b5371
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/582b5371
Branch: refs/heads/develop
Commit: 582b53716057781861c2ae49e754a51add4ef3d0
Parents: b6be5d4
Author: Hitesh Khamesra <hk...@pivotal.io>
Authored: Wed Apr 20 16:00:06 2016 -0700
Committer: Hitesh Khamesra <hk...@pivotal.io>
Committed: Fri Apr 22 10:38:08 2016 -0700
----------------------------------------------------------------------
.../membership/gms/membership/GMSJoinLeave.java | 34 +++++++++-----------
.../gms/membership/GMSJoinLeaveJUnitTest.java | 7 +++-
2 files changed, 22 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/582b5371/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
index 8b54838..05350e5 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
@@ -389,18 +389,9 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
}
throw new AuthenticationFailedException(failReason);
}
-
- if (response.getCurrentView() == null) {
- logger.info("received join response with no membership view: {}", response);
- return isJoined;
- }
-
- this.birthViewId = response.getMemberID().getVmViewId();
- this.localAddress.setVmViewId(this.birthViewId);
- GMSMember me = (GMSMember) this.localAddress.getNetMember();
- me.setBirthViewId(birthViewId);
- installView(response.getCurrentView());
- return true;
+
+ //there is no way we can rech here right now
+ throw new RuntimeException("Join Request Failed with response " + joinResponse );
}
private JoinResponseMessage waitForJoinResponse() throws InterruptedException {
@@ -759,11 +750,16 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
return newView;
}
- private void sendRemoveMessages(List<InternalDistributedMember> removals, List<String> reasons, NetView newView) {
+ private void sendRemoveMessages(List<InternalDistributedMember> removals, List<String> reasons, NetView newView, Set<InternalDistributedMember> oldIds) {
Iterator<String> reason = reasons.iterator();
for (InternalDistributedMember mbr : removals) {
- RemoveMemberMessage response = new RemoveMemberMessage(mbr, mbr, reason.next());
- services.getMessenger().send(response);
+ //if olds not contains mbr then send remove request
+ if (!oldIds.contains(mbr)) {
+ RemoveMemberMessage response = new RemoveMemberMessage(mbr, mbr, reason.next());
+ services.getMessenger().send(response);
+ } else {
+ reason.next();
+ }
}
}
@@ -2091,7 +2087,9 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
removalReasons.add(((RemoveMemberMessage) msg).getReason());
} else {
sendRemoveMessages(Collections.<InternalDistributedMember>singletonList(mbr),
- Collections.<String>singletonList(((RemoveMemberMessage) msg).getReason()), currentView);
+ Collections.<String>singletonList(((RemoveMemberMessage) msg).getReason()),
+ currentView,
+ new HashSet<InternalDistributedMember>());
}
}
break;
@@ -2154,7 +2152,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
}
// send removal messages before installing the view so we stop
// getting messages from members that have been kicked out
- sendRemoveMessages(removalReqs, removalReasons, newView);
+ sendRemoveMessages(removalReqs, removalReasons, newView, oldIDs);
prepareAndSendView(newView, joinReqs, leaveReqs, newView.getCrashedMembers());
@@ -2265,7 +2263,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
for (int i = 0; i < size; i++) {
reasons.add("Failed to acknowledge a new membership view and then failed tcp/ip connection attempt");
}
- sendRemoveMessages(failures, reasons, newView);
+ sendRemoveMessages(failures, reasons, newView, new HashSet<InternalDistributedMember>());
}
// if there is no conflicting view then we can count
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/582b5371/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
index 6c23037..50bed13 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
@@ -1075,19 +1075,24 @@ public class GMSJoinLeaveJUnitTest {
FindCoordinatorResponse fcr = new FindCoordinatorResponse(mockMembers[0], mockMembers[0], false, null, registrants, false, true);
NetView view = createView();
JoinResponseMessage jrm = new JoinResponseMessage(mockMembers[0], view);
- gmsJoinLeave.setJoinResponseMessage(jrm);
TcpClientWrapper tcpClientWrapper = mock(TcpClientWrapper.class);
gmsJoinLeave.setTcpClientWrapper(tcpClientWrapper);
FindCoordinatorRequest fcreq = new FindCoordinatorRequest(gmsJoinLeaveMemberId, new HashSet<>(), -1);
int connectTimeout = (int)services.getConfig().getMemberTimeout() * 2;
when(tcpClientWrapper.sendCoordinatorFindRequest(new InetSocketAddress("localhost", 12345), fcreq, connectTimeout)).thenReturn(fcr);
+ callAsnyc(()->{gmsJoinLeave.installView(view);});
assertTrue("Should be able to join ", gmsJoinLeave.join());
}finally{
}
}
+ private void callAsnyc(Runnable run) {
+ Thread th = new Thread(run);
+ th.start();
+ }
+
@Test
public void testCoordinatorFindRequestFailure() throws Exception {
try{
[2/2] incubator-geode git commit: GEODE-1150 Find Coodinator response
was not notifying waiting thread
Posted by hi...@apache.org.
GEODE-1150 Find Coodinator response was not notifying waiting thread
1. Now find Coordinator response notifies waiting thread.
2. joinrequest sees join response if member is no more coordinator
3. Test issue - now we wait for coordinator to change before verifying it
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/b6be5d47
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/b6be5d47
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/b6be5d47
Branch: refs/heads/develop
Commit: b6be5d473df2c79a8527f688c6ee875337b96409
Parents: 70c5467
Author: Hitesh Khamesra <hk...@pivotal.io>
Authored: Mon Apr 18 09:41:41 2016 -0700
Committer: Hitesh Khamesra <hk...@pivotal.io>
Committed: Fri Apr 22 10:38:08 2016 -0700
----------------------------------------------------------------------
.../membership/gms/membership/GMSJoinLeave.java | 62 +++++++++++++++++++-
.../gemfire/distributed/LocatorDUnitTest.java | 32 ++++++----
.../gms/membership/GMSJoinLeaveTestHelper.java | 5 ++
3 files changed, 85 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b6be5d47/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
index d91b247..8b54838 100755
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
@@ -414,6 +414,29 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
joinResponse.wait(timeout);
}
response = joinResponse[0];
+
+ if (response != null && response.getCurrentView() != null && !isJoined) {
+ //reset joinResponse[0]
+ joinResponse[0] = null;
+ // we got view here that means either we have to wait for
+ NetView v = response.getCurrentView();
+ InternalDistributedMember coord = v.getCoordinator();
+ if (searchState.alreadyTried.contains(coord)) {
+ // we already sent join request to it..so lets wait some more time here
+ // assuming we got this response immediately, so wait for same timeout here..
+ long timeout = Math.max(services.getConfig().getMemberTimeout(), services.getConfig().getJoinTimeout() / 5);
+ joinResponse.wait(timeout);
+ response = joinResponse[0];
+ } else {
+ // try on this coordinator
+ searchState.possibleCoordinator = coord;
+ response = null;
+ }
+ searchState.view = v;
+ }
+ if (isJoined) {
+ return null;
+ }
}
return response;
}
@@ -620,6 +643,9 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
viewRequests.add(request);
viewRequests.notifyAll();
}
+ if (viewCreator != null) {
+ viewCreator.informToPendingJoinRequests();
+ }
}
// for testing purposes, returns a copy of the view requests for verification
@@ -1174,6 +1200,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
private void processFindCoordinatorResponse(FindCoordinatorResponse resp) {
synchronized (searchState.responses) {
searchState.responses.add(resp);
+ searchState.responses.notifyAll();
}
}
@@ -1771,7 +1798,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
}
class ViewCreator extends Thread {
- boolean shutdown = false;
+ volatile boolean shutdown = false;
volatile boolean waiting = false;
volatile boolean testFlagForRemovalRequest = false;
@@ -1960,6 +1987,39 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
}
} finally {
shutdown = true;
+ informToPendingJoinRequests();
+ }
+ }
+
+ synchronized void informToPendingJoinRequests() {
+ if (!shutdown) {
+ return;
+ }
+
+ ArrayList<DistributionMessage> requests = new ArrayList<>();
+ synchronized (viewRequests) {
+ if (viewRequests.size() > 0) {
+ requests.addAll(viewRequests);
+ } else {
+ return;
+ }
+ viewRequests.clear();
+ }
+
+ for (DistributionMessage msg : requests) {
+ switch (msg.getDSFID()) {
+ case JOIN_REQUEST:
+ logger.info("Informing to pending join requests {}", msg);
+
+ NetView v = currentView;
+ if (!v.getCoordinator().equals(localAddress)) {
+ //lets inform that coordinator has been changed
+ JoinResponseMessage jrm = new JoinResponseMessage(((JoinRequestMessage) msg).getMemberID(), v);
+ services.getMessenger().send(jrm);
+ }
+ default:
+ break;
+ }
}
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b6be5d47/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 71eb68e..702a859 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
@@ -1412,7 +1412,6 @@ public class LocatorDUnitTest extends DistributedTestCase {
}
};
}
-
/**
* Tests starting multiple locators at the same time and ensuring that the locators
* end up only have 1 master.
@@ -1516,9 +1515,11 @@ public class LocatorDUnitTest extends DistributedTestCase {
host0 + "[" + port3 + "]";
dsProps.setProperty("locators", newLocators);
- assertTrue(vm3.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ assertTrue(vm3.invoke("Checking ViewCreator thread on Lead Server", () -> GMSJoinLeaveTestHelper.isViewCreator()));
//Given the start up order of servers, this server is the elder server
- assertTrue(vm3.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ assertTrue(vm3.invoke("Checking ViewCreator thread on Lead Server", () -> GMSJoinLeaveTestHelper.isViewCreator()));
+
+ final InternalDistributedMember currentCoordinator = GMSJoinLeaveTestHelper.getCurrentCoordinator();
startLocatorAsync(vm1, new Object[] { port2, dsProps });
startLocatorAsync(vm2, new Object[] { port3, dsProps });
@@ -1526,6 +1527,11 @@ public class LocatorDUnitTest extends DistributedTestCase {
waitCriterion = new WaitCriterion() {
public boolean done() {
try {
+ InternalDistributedMember c = GMSJoinLeaveTestHelper.getCurrentCoordinator();
+ if (c.equals(currentCoordinator)) {
+ //now locator should be new coordinator
+ return false;
+ }
return system.getDM().getAllHostedLocators().size() == 2;
} catch (Exception e) {
e.printStackTrace();
@@ -1540,17 +1546,17 @@ public class LocatorDUnitTest extends DistributedTestCase {
};
Wait.waitForCriterion(waitCriterion, 15 * 1000, 200, true);
- int netviewId = vm1.invoke(() -> GMSJoinLeaveTestHelper.getViewId());
- assertEquals(netviewId, (int) vm2.invoke(() -> GMSJoinLeaveTestHelper.getViewId()));
- assertEquals(netviewId, (int) vm3.invoke(() -> GMSJoinLeaveTestHelper.getViewId()));
- assertEquals(netviewId, (int) vm4.invoke(() -> GMSJoinLeaveTestHelper.getViewId()));
- assertFalse(vm4.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ int netviewId = vm1.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.getViewId());
+ assertEquals(netviewId, (int) vm2.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId()));
+ assertEquals(netviewId, (int) vm3.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId()));
+ assertEquals(netviewId, (int) vm4.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId()));
+ assertFalse(vm4.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator()));
//Given the start up order of servers, this server is the elder server
- assertFalse(vm3.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ assertFalse(vm3.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator()));
if (vm1.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())) {
- assertFalse(vm2.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ assertFalse(vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator()));
} else {
- assertTrue(vm2.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator()));
+ assertTrue(vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator()));
}
} finally {
@@ -1565,7 +1571,7 @@ public class LocatorDUnitTest extends DistributedTestCase {
}
private void startLocatorSync(VM vm, Object[] args) {
- vm.invoke(new SerializableRunnable("Starting process on " + args[0], args) {
+ vm.invoke(new SerializableRunnable("Starting locator process on " + args[0], args) {
public void run() {
File logFile = new File("");
try {
@@ -1578,7 +1584,7 @@ public class LocatorDUnitTest extends DistributedTestCase {
}
private void startLocatorAsync(VM vm, Object[] args) {
- vm.invokeAsync(new SerializableRunnable("Starting process on " + args[0], args) {
+ vm.invokeAsync(new SerializableRunnable("Starting Locator process async on " + args[0], args) {
public void run() {
File logFile = new File("");
try {
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b6be5d47/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
index 17409a4..bf13420 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
@@ -19,6 +19,7 @@ package com.gemstone.gemfire.distributed.internal.membership.gms.membership;
import com.gemstone.gemfire.distributed.Locator;
import com.gemstone.gemfire.distributed.internal.DM;
import com.gemstone.gemfire.distributed.internal.InternalDistributedSystem;
+import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember;
import com.gemstone.gemfire.distributed.internal.membership.gms.Services;
import com.gemstone.gemfire.distributed.internal.membership.gms.mgr.GMSMembershipManager;
import com.gemstone.gemfire.test.dunit.Wait;
@@ -70,6 +71,10 @@ public class GMSJoinLeaveTestHelper {
Services services = membershipManager.getServices();
return (GMSJoinLeave) services.getJoinLeave();
}
+
+ public static InternalDistributedMember getCurrentCoordinator() {
+ return getGmsJoinLeave().getView().getCoordinator();
+ }
public static Integer getViewId() {
return getGmsJoinLeave().getView().getViewId();