You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nn...@apache.org on 2021/07/19 16:56:06 UTC

[geode] branch support/1.14 updated: GEODE-9350: MemberJoinedEvent should be triggered after new view is installed (#6598) (#6695)

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

nnag pushed a commit to branch support/1.14
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.14 by this push:
     new e576923  GEODE-9350: MemberJoinedEvent should be triggered after new view is installed (#6598) (#6695)
e576923 is described below

commit e5769236f385142468df87b67aa25cb3a6337b9f
Author: Kamilla Aslami <ka...@vmware.com>
AuthorDate: Mon Jul 19 09:54:20 2021 -0700

    GEODE-9350: MemberJoinedEvent should be triggered after new view is installed (#6598) (#6695)
    
    * GEODE-9350: trigger MemberJoinedEvent after the new view is installed
    
    (cherry picked from commit b585039a7f7c71f7ed26b6d5b4a5d3561309c19d)
---
 .../distributed/DistributedSystemDUnitTest.java    | 29 ----------------
 .../distributed/FailDeserializationFunction.java   | 40 ++++++++++++++++++++++
 .../ClusterDistributionManagerDUnitTest.java       | 27 +++++++++++++++
 .../internal/membership/gms/GMSMembership.java     | 14 ++++----
 4 files changed, 74 insertions(+), 36 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java
index 3c02b7f..a318cef 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java
@@ -39,9 +39,6 @@ import static org.apache.geode.test.dunit.LogWriterUtils.getLogWriter;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
-import java.io.DataInput;
-import java.io.DataOutput;
-import java.io.IOException;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -58,7 +55,6 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.CancelException;
-import org.apache.geode.DataSerializable;
 import org.apache.geode.GemFireConfigException;
 import org.apache.geode.SerializationException;
 import org.apache.geode.SystemConnectException;
@@ -66,7 +62,6 @@ import org.apache.geode.cache.AttributesFactory;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.Region;
-import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.FunctionService;
 import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.Distribution;
@@ -461,28 +456,4 @@ public class DistributedSystemDUnitTest extends JUnit4DistributedTestCase {
       return "FakeMessage(blocking=" + (this.blocked != null) + ")";
     }
   }
-
-  /**
-   * A function that cannot be deserialized, used for failure handling
-   */
-  public static class FailDeserializationFunction
-      implements org.apache.geode.cache.execute.Function,
-      DataSerializable {
-    @Override
-    public void execute(FunctionContext context) {
-
-    }
-
-
-    @Override
-    public void toData(DataOutput out) throws IOException {
-
-    }
-
-    @Override
-    public void fromData(DataInput in) throws IOException, ClassNotFoundException {
-      throw new ClassNotFoundException("Fake class not found");
-
-    }
-  }
 }
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/distributed/FailDeserializationFunction.java b/geode-core/src/distributedTest/java/org/apache/geode/distributed/FailDeserializationFunction.java
new file mode 100644
index 0000000..8470235
--- /dev/null
+++ b/geode-core/src/distributedTest/java/org/apache/geode/distributed/FailDeserializationFunction.java
@@ -0,0 +1,40 @@
+/*
+ * 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;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+
+import org.apache.geode.DataSerializable;
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.FunctionContext;
+
+/**
+ * A function that cannot be deserialized, used for failure handling
+ */
+public class FailDeserializationFunction implements Function, DataSerializable {
+  @Override
+  public void execute(FunctionContext context) {}
+
+  @Override
+  public void toData(DataOutput out) throws IOException {}
+
+  @Override
+  public void fromData(DataInput in) throws IOException, ClassNotFoundException {
+    throw new ClassNotFoundException("Fake class not found");
+  }
+}
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/ClusterDistributionManagerDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/ClusterDistributionManagerDUnitTest.java
index 78a9a89..d854aaa 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/ClusterDistributionManagerDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/ClusterDistributionManagerDUnitTest.java
@@ -65,9 +65,11 @@ import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionEvent;
 import org.apache.geode.cache.RegionFactory;
 import org.apache.geode.cache.Scope;
+import org.apache.geode.cache.execute.FunctionService;
 import org.apache.geode.cache.util.CacheListenerAdapter;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.FailDeserializationFunction;
 import org.apache.geode.distributed.Locator;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.distributed.internal.membership.api.MemberDisconnectedException;
@@ -99,6 +101,7 @@ public class ClusterDistributionManagerDUnitTest extends CacheTestCase {
 
   private VM locatorvm;
   private VM vm1;
+  private VM vm2;
 
   @Rule
   public DistributedRestoreSystemProperties restoreSystemProperties =
@@ -114,12 +117,14 @@ public class ClusterDistributionManagerDUnitTest extends CacheTestCase {
 
     locatorvm = VM.getVM(0);
     vm1 = VM.getVM(1);
+    vm2 = VM.getVM(2);
     Invoke.invokeInEveryVM(() -> System.setProperty("p2p.joinTimeout", "120000"));
     final int port = locatorvm.invoke(() -> {
       System.setProperty(BYPASS_DISCOVERY_PROPERTY, "true");
       return Locator.startLocatorAndDS(0, new File(""), new Properties()).getPort();
     });
     vm1.invoke(() -> locatorPort = port);
+    vm2.invoke(() -> locatorPort = port);
     locatorPort = port;
   }
 
@@ -465,6 +470,28 @@ public class ClusterDistributionManagerDUnitTest extends CacheTestCase {
     future.get(getTimeout().toMillis(), TimeUnit.MILLISECONDS);
   }
 
+  @Test
+  public void testMemberJoinsAfterNewViewIsInstalled() {
+    vm1.invoke("join the cluster", () -> getSystem().getDistributedMember());
+    vm1.invoke(
+        () -> system.getDistributionManager().addMembershipListener(new MembershipListener() {
+          @Override
+          public void memberJoined(DistributionManager distributionManager,
+              InternalDistributedMember id) {
+            if (!distributionManager.getDistribution().getMembership().memberExists(id)) {
+              // if MemberJoinedEvent is triggered before the new view is installed,
+              // the test will throw an exception
+              FunctionService.onMember(id)
+                  .execute(new FailDeserializationFunction());
+            }
+          }
+        }));
+    // trigger MemberJoinedEvent
+    DistributedMember member =
+        vm2.invoke("join the cluster", () -> getSystem().getDistributedMember());
+    assertThat(member).isNotNull();
+  }
+
   private CacheListener<String, String> getSleepingListener(final boolean playDead) {
     regionDestroyedInvoked = false;
 
diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
index f9daece..ff9a383 100644
--- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
+++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
@@ -369,11 +369,10 @@ public class GMSMembership<ID extends MemberIdentifier> implements Membership<ID
       // update the view to reflect our changes, so that
       // callbacks will see the new (updated) view.
       MembershipView<ID> newlatestView = newView;
+      final List<ID> newMembers = new ArrayList<>();
 
       // look for additions
-      for (int i = 0; i < newView.getMembers().size(); i++) { // additions
-        ID m = newView.getMembers().get(i);
-
+      for (ID m : newView.getMembers()) { // additions
         // Once a member has been seen via a view, remove them from the
         // newborn set. Replace the member data of the surpriseMember ID
         // in case it was a partial ID and is being retained by DistributionManager
@@ -416,13 +415,11 @@ public class GMSMembership<ID extends MemberIdentifier> implements Membership<ID
         }
 
         logger.info("Membership: Processing addition <{}>", m);
-
-        listener.newMemberConnected(m);
+        newMembers.add(m);
       } // additions
 
       // look for departures
-      for (int i = 0; i < priorView.getMembers().size(); i++) { // departures
-        ID m = priorView.getMembers().get(i);
+      for (ID m : priorView.getMembers()) { // departures
         if (newView.contains(m)) {
           continue; // still alive
         }
@@ -479,6 +476,9 @@ public class GMSMembership<ID extends MemberIdentifier> implements Membership<ID
 
       // the view is complete - let's install it
       latestView = newlatestView;
+      for (ID newMember : newMembers) {
+        listener.newMemberConnected(newMember);
+      }
       listener.viewInstalled(latestView);
     } finally {
       latestViewWriteLock.unlock();