Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: klund@apache.org
To: commits@geode.incubator.apache.org
Date: Fri, 11 Dec 2015 21:22:35 -0000
Message-Id: <696653637cf04b7db23d9c6088fc9419@git.apache.org>
Subject: [01/50] [abbrv] incubator-geode git commit: GEODE-616: failure
 detection ports not set in new membership view

Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-217 3733ae71b -> 0e62bc04b


GEODE-616: failure detection ports not set in new membership view

Added a method to NetView to copy failure detection ports from an old
view to the new view.  I also added a JUnit test for NetView to make
sure we have adequate code-coverage for this class.


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

Branch: refs/heads/feature/GEODE-217
Commit: e4448adc3ce1c5774410194dcbd361eb21890e7d
Parents: 2803a10
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Tue Dec 1 08:33:02 2015 -0800
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Tue Dec 1 08:33:02 2015 -0800

----------------------------------------------------------------------
 .../internal/membership/NetView.java            |  46 ++++-
 .../membership/gms/membership/GMSJoinLeave.java |  25 +--
 .../internal/membership/NetViewJUnitTest.java   | 194 +++++++++++++++++++
 3 files changed, 241 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e4448adc/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
index 8800d9d..a90a45d 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
@@ -88,6 +88,12 @@ public class NetView implements DataSerializableFixedID {
     Arrays.fill(failureDetectionPorts, -1);
   }
 
+  /**
+   * Create a new view with the contents of the given view and the
+   * specified view ID
+   * @param other
+   * @param viewId
+   */
   public NetView(NetView other, int viewId) {
     this.creator = other.creator;
     this.viewId = viewId;
@@ -129,7 +135,7 @@ public class NetView implements DataSerializableFixedID {
   
   public int getFailureDetectionPort(InternalDistributedMember mbr) {
     int idx = members.indexOf(mbr);
-    if (idx < 0 || failureDetectionPorts == null || idx >= failureDetectionPorts.length) {
+    if (idx < 0 || idx >= failureDetectionPorts.length) {
       return -1;
     }
     return failureDetectionPorts[idx];
@@ -143,15 +149,36 @@ public class NetView implements DataSerializableFixedID {
     ensureFDCapacity(idx);
     failureDetectionPorts[idx] = port;
   }
+  
+  /**
+   * Transfer the failure-detection ports from another view to this one
+   * @param otherView
+   */
+  public void setFailureDetectionPorts(NetView otherView) {
+    int[] ports = otherView.getFailureDetectionPorts();
+    if (ports != null) {
+      int idx = 0;
+      int portsSize = ports.length;
+      for (InternalDistributedMember mbr: otherView.getMembers()) {
+        if (contains(mbr)) {
+          // unit tests create views w/o failure detection ports, so we must check the length
+          // of the array
+          if (idx < portsSize) {
+            setFailureDetectionPort(mbr, ports[idx]);
+          } else {
+            setFailureDetectionPort(mbr, -1);
+          }
+        }
+        idx += 1;
+      }
+    }
+  }
 
   /**
    * ensures that there is a slot at idx to store an int
    */
   private void ensureFDCapacity(int idx) {
-    if (failureDetectionPorts == null) {
-      failureDetectionPorts = new int[idx+10];
-      Arrays.fill(failureDetectionPorts, -1);
-    } else if (idx >= failureDetectionPorts.length) {
+    if (idx >= failureDetectionPorts.length) {
       int[] p = new int[idx+10];
       if (failureDetectionPorts.length > 0) {
         System.arraycopy(failureDetectionPorts, 0, p, 0, failureDetectionPorts.length);
@@ -479,6 +506,15 @@ public class NetView implements DataSerializableFixedID {
         first = false;
       }
     }
+//    sb.append("] fd ports: [");
+//    int[] ports = getFailureDetectionPorts();
+//    int numMembers = size();
+//    for (int i=0; i<numMembers; i++) {
+//      if (i > 0) {
+//        sb.append(' ');
+//      }
+//      sb.append(ports[i]);
+//    }
     sb.append("]");
     return sb.toString();
   }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e4448adc/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
index 1c7b601..2f8d734 100755
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
@@ -531,7 +531,6 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
       NetView check = new NetView(v, v.getViewId() + 1);
       synchronized (removedMembers) {
         removedMembers.add(mbr);
-        check = new NetView(v, v.getViewId());
         check.addCrashedMembers(removedMembers);
         check.removeAll(removedMembers);
       }
@@ -660,6 +659,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
         mbrs.removeAll(leaving);
         newView = new NetView(this.localAddress, viewNumber, mbrs, leaving,
             removals);
+        newView.setFailureDetectionPorts(currentView);
         newView.setFailureDetectionPort(this.localAddress, services.getHealthMonitor().getFailureDetectionPort());
       }
       if (viewCreator == null || viewCreator.isShutdown()) {
@@ -1890,23 +1890,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
           }
         }
         if (currentView != null) {
-          int[] ports = currentView.getFailureDetectionPorts();
-          if (ports != null) {
-            int idx = 0;
-            int portsSize = ports.length;
-            for (InternalDistributedMember mbr: currentView.getMembers()) {
-              if (newView.contains(mbr)) {
-                // unit tests create views w/o failure detection ports, so we must check the length
-                // of the array
-                if (idx < portsSize) {
-                  newView.setFailureDetectionPort(mbr, ports[idx]);
-                } else {
-                  newView.setFailureDetectionPort(mbr, -1);
-                }
-              }
-              idx += 1;
-            }
-          }
+          newView.setFailureDetectionPorts(currentView);
         }
       }
 
@@ -1995,12 +1979,15 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler {
             logger.info("adding these new members from a conflicting view to the new view: {}", newMembers);
             for (InternalDistributedMember mbr: newMembers) {
               int port = conflictingView.getFailureDetectionPort(mbr);
-              JoinRequestMessage msg = new JoinRequestMessage(localAddress, mbr, null, port);
               newView.add(mbr);
               newView.setFailureDetectionPort(mbr, port);
               joinReqs.add(mbr);
             }
           }
+          // trump the view ID of the conflicting view so mine will be accepted
+          if (conflictingView.getViewId() >= newView.getViewId()) {
+            newView = new NetView(newView, conflictingView.getViewId()+1);
+          }
         }
 
         if (!unresponsive.isEmpty()) {

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e4448adc/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/NetViewJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/NetViewJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/NetViewJUnitTest.java
new file mode 100755
index 0000000..603c7bf
--- /dev/null
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/NetViewJUnitTest.java
@@ -0,0 +1,194 @@
+package com.gemstone.gemfire.distributed.internal.membership;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Set;
+import java.util.Timer;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import com.gemstone.gemfire.distributed.internal.DistributionManager;
+import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember;
+import com.gemstone.gemfire.distributed.internal.membership.NetView;
+import com.gemstone.gemfire.internal.SocketCreator;
+import com.gemstone.gemfire.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class NetViewJUnitTest {
+  List<InternalDistributedMember> members;
+  
+  @Before
+  public void initMembers() throws Exception {
+    int numMembers = 10;
+    members = new ArrayList<>(numMembers);
+    for (int i=0; i<numMembers; i++) {
+      members.add(new InternalDistributedMember(SocketCreator.getLocalHost(), 1000+i));
+    }
+    // view creator is a locator
+    members.get(0).setVmKind(DistributionManager.LOCATOR_DM_TYPE);
+    members.get(0).setVmViewId(0);
+    members.get(0).getNetMember().setPreferredForCoordinator(true);
+    
+    // members who joined in view #1
+    for (int i=1; i<(numMembers-1); i++) {
+      members.get(i).setVmViewId(1);
+      members.get(i).setVmKind(DistributionManager.NORMAL_DM_TYPE);
+      members.get(i).getNetMember().setPreferredForCoordinator(false);
+    }
+
+    // member joining in this view
+    members.get(numMembers-1).setVmViewId(2);
+    members.get(numMembers-1).setVmKind(DistributionManager.NORMAL_DM_TYPE);
+  }
+  
+  private void setFailureDetectionPorts(NetView view) {
+    int numMembers = members.size();
+    // use the membership port as the FD port so it's easy to figure out problems
+    for (int i=0; i<numMembers; i++) {
+      view.setFailureDetectionPort(members.get(i), members.get(i).getPort());
+    }
+  }
+  
+  @Test
+  public void testCreateView() throws Exception {
+    int numMembers = members.size();
+    NetView view = new NetView(members.get(0), 2, members, Collections.emptySet(), Collections.emptySet());
+    setFailureDetectionPorts(view);
+    
+    assertTrue(view.getCreator().equals(members.get(0)));
+    assertEquals(2, view.getViewId());
+    assertEquals(members, view.getMembers());
+    assertEquals(0, view.getCrashedMembers().size());
+    assertEquals(members.get(1), view.getLeadMember()); // a locator can't be lead member
+    assertEquals(0, view.getShutdownMembers().size());
+    assertEquals(1, view.getNewMembers().size());
+    assertEquals(members.get(numMembers-1), view.getNewMembers().iterator().next());
+    assertEquals(members.get(0), view.getCoordinator());
+    
+    for (int i=0; i<numMembers; i++) {
+      InternalDistributedMember mbr = members.get(i);
+      assertEquals(mbr.getPort(), view.getFailureDetectionPort(mbr));
+    }
+    
+    assertFalse(view.shouldBeCoordinator(members.get(1)));
+    assertTrue(view.shouldBeCoordinator(members.get(0)));
+    assertEquals(members.get(numMembers-1), view.getCoordinator(Collections.singletonList(members.get(0))));
+    members.get(numMembers-1).getNetMember().setPreferredForCoordinator(false);
+    assertEquals(members.get(1), view.getCoordinator(Collections.singletonList(members.get(0))));
+    
+    members.get(numMembers-1).getNetMember().setPreferredForCoordinator(true);
+    List<InternalDistributedMember> preferred = view.getPreferredCoordinators(Collections.<InternalDistributedMember>singleton(members.get(1)), members.get(0), 2);
+    assertEquals(3, preferred.size());
+    assertEquals(members.get(numMembers-1), preferred.get(0));
+  }
+  
+  @Test
+  public void testRemoveMembers() throws Exception {
+    int numMembers = members.size();
+    NetView view = new NetView(members.get(0), 2, new ArrayList<>(members), Collections.emptySet(),
+        Collections.emptySet());
+    setFailureDetectionPorts(view);
+
+    for (int i=1; i<numMembers; i+=2) {
+      view.remove(members.get(i));
+      assertFalse(view.contains(members.get(i)));
+    }
+    
+    List<InternalDistributedMember> remainingMembers = view.getMembers();
+    int num = remainingMembers.size();
+    for (int i=0; i<num; i++) {
+      InternalDistributedMember mbr = remainingMembers.get(i);
+      assertEquals(mbr.getPort(), view.getFailureDetectionPort(mbr));
+    }
+  }
+  
+  @Test
+  public void testRemoveAll() throws Exception {
+    int numMembers = members.size();
+    NetView view = new NetView(members.get(0), 2, new ArrayList<>(members), Collections.emptySet(),
+        Collections.emptySet());
+    setFailureDetectionPorts(view);
+
+    Collection<InternalDistributedMember> removals = new ArrayList<>(numMembers/2);
+    for (int i=1; i<numMembers; i+=2) {
+      removals.add(members.get(i));
+    }
+    
+    view.removeAll(removals);
+    for (InternalDistributedMember mbr: removals) {
+      assertFalse(view.contains(mbr));
+    }
+    assertEquals(numMembers-removals.size(), view.size());
+
+    List<InternalDistributedMember> remainingMembers = view.getMembers();
+    int num = remainingMembers.size();
+    for (int i=0; i<num; i++) {
+      InternalDistributedMember mbr = remainingMembers.get(i);
+      assertEquals(mbr.getPort(), view.getFailureDetectionPort(mbr));
+    }
+  }
+  
+  @Test
+  public void testCopyView() throws Exception {
+    NetView view = new NetView(members.get(0), 2, new ArrayList<>(members), Collections.emptySet(),
+        Collections.emptySet());
+    setFailureDetectionPorts(view);
+
+    NetView newView = new NetView(view, 3);
+
+    assertTrue(newView.getCreator().equals(members.get(0)));
+    assertEquals(3, newView.getViewId());
+    assertEquals(members, newView.getMembers());
+    assertEquals(0, newView.getCrashedMembers().size());
+    assertEquals(members.get(1), newView.getLeadMember()); // a locator can't be lead member
+    assertEquals(0, newView.getShutdownMembers().size());
+    assertEquals(0, newView.getNewMembers().size());
+    assertTrue(newView.equals(view));
+    assertTrue(view.equals(newView));
+    newView.remove(members.get(1));
+    assertFalse(newView.equals(view));
+  }
+  
+  @Test
+  public void testAddLotsOfMembers() throws Exception {
+    NetView view = new NetView(members.get(0), 2, new ArrayList<>(members), Collections.emptySet(),
+        Collections.emptySet());
+    setFailureDetectionPorts(view);
+    
+    NetView copy = new NetView(view, 2);
+
+    int oldSize = view.size();
+    for (int i=0; i<100; i++) {
+      InternalDistributedMember mbr = new InternalDistributedMember(SocketCreator.getLocalHost(), 2000+i);
+      mbr.setVmKind(DistributionManager.NORMAL_DM_TYPE);
+      mbr.setVmViewId(2);
+      view.add(mbr);
+      view.setFailureDetectionPort(mbr, 2000+i);
+    }
+    
+    assertEquals(oldSize+100, view.size());
+    for (InternalDistributedMember mbr: view.getMembers()) {
+      assertEquals(mbr.getPort(), view.getFailureDetectionPort(mbr));
+    }
+    
+    assertEquals(100, view.getNewMembers(copy).size());
+  }
+  
+}