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 2019/12/05 17:50:09 UTC

[geode] branch feature/GEODE-7551 created (now 461f6da)

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

bschuchardt pushed a change to branch feature/GEODE-7551
in repository https://gitbox.apache.org/repos/asf/geode.git.


      at 461f6da  GEODE-7551: Remove membership API dependency on ClusterDistributionManager

This branch includes the following new commits:

     new 461f6da  GEODE-7551: Remove membership API dependency on ClusterDistributionManager

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[geode] 01/01: GEODE-7551: Remove membership API dependency on ClusterDistributionManager

Posted by bs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-7551
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 461f6da508ffed90bb752f193440f16a761f7d59
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Thu Dec 5 08:57:24 2019 -0800

    GEODE-7551: Remove membership API dependency on ClusterDistributionManager
    
    ClusterDistributionManager was only being used to get canonical IDs and
    to check for shutdown conditions.  I've modified the CDM to set shutdown
    status in membership in its shutdown() method and have modified
    membership to just use the current MembershipView (which contains any
    surprise members) instead of asking CDM for a canonical ID, which just
    grabs said MembershipView and does the same thing.
---
 .../internal/membership/MembershipJUnitTest.java   |  2 +-
 .../membership/gms/GMSMembershipJUnitTest.java     |  6 +----
 .../internal/ClusterDistributionManager.java       |  1 +
 .../distributed/internal/DistributionImpl.java     |  3 +--
 .../internal/membership/gms/GMSMembership.java     | 28 ++++++----------------
 .../membership/gms/MembershipBuilderImpl.java      |  8 ++-----
 .../membership/gms/api/MemberIdentifier.java       |  5 ++++
 .../membership/gms/api/MembershipBuilder.java      |  5 ++--
 .../membership/gms/interfaces/Manager.java         |  6 -----
 .../membership/gms/membership/GMSJoinLeave.java    |  2 +-
 .../gms/api/MembershipAPIArchUnitTest.java         |  3 ---
 11 files changed, 21 insertions(+), 48 deletions(-)

diff --git a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
index 7f2cda1..5622252 100755
--- a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
@@ -271,7 +271,7 @@ public class MembershipJUnitTest {
     });
     LifecycleListener lifeCycleListener = mock(LifecycleListener.class);
     final Membership m1 =
-        MembershipBuilder.newMembershipBuilder(null)
+        MembershipBuilder.newMembershipBuilder()
             .setAuthenticator(new GMSAuthenticator(config.getSecurityProps(), securityService,
                 mockSystem.getSecurityLogWriter(), mockSystem.getInternalLogWriter()))
             .setStatistics(stats1)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipJUnitTest.java
index 7453631..dc8bca4 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipJUnitTest.java
@@ -51,7 +51,6 @@ import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
 import org.apache.geode.distributed.internal.HighPriorityAckedMessage;
-import org.apache.geode.distributed.internal.direct.DirectChannel;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.distributed.internal.membership.MembershipView;
 import org.apache.geode.distributed.internal.membership.adapter.LocalViewMessage;
@@ -79,7 +78,6 @@ public class GMSMembershipJUnitTest {
   private Services services;
   private MembershipConfig mockConfig;
   private DistributionConfig distConfig;
-  private Properties distProperties;
   private Authenticator authenticator;
   private HealthMonitor healthMonitor;
   private InternalDistributedMember myMemberId;
@@ -90,7 +88,6 @@ public class GMSMembershipJUnitTest {
   private MembershipListener listener;
   private GMSMembership manager;
   private List<InternalDistributedMember> members;
-  private DirectChannel dc;
   private MessageListener messageListener;
   private LifecycleListener directChannelCallback;
 
@@ -107,7 +104,6 @@ public class GMSMembershipJUnitTest {
     nonDefault.put(MEMBER_TIMEOUT, "2000");
     nonDefault.put(LOCATORS, "localhost[10344]");
     distConfig = new DistributionConfigImpl(nonDefault);
-    distProperties = nonDefault;
     RemoteTransportConfig tconfig =
         new RemoteTransportConfig(distConfig, ClusterDistributionManager.NORMAL_DM_TYPE);
 
@@ -154,7 +150,7 @@ public class GMSMembershipJUnitTest {
     listener = mock(MembershipListener.class);
     messageListener = mock(MessageListener.class);
     directChannelCallback = mock(LifecycleListener.class);
-    manager = new GMSMembership(listener, messageListener, null, directChannelCallback);
+    manager = new GMSMembership(listener, messageListener, directChannelCallback);
     manager.getGMSManager().init(services);
     when(services.getManager()).thenReturn(manager.getGMSManager());
   }
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
index f18c064..1f632e2 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
@@ -1089,6 +1089,7 @@ public class ClusterDistributionManager implements DistributionManager {
         return;
       }
       closeInProgress = true;
+      this.distribution.setShutdown();
     } // synchronized
 
     // [bruce] log shutdown at info level and with ID to balance the
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
index 5e18516..55b934b 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionImpl.java
@@ -123,8 +123,7 @@ public class DistributionImpl implements Distribution {
     }
 
     memberTimeout = system.getConfig().getMemberTimeout();
-    membership = MembershipBuilder.newMembershipBuilder(
-        clusterDistributionManager)
+    membership = MembershipBuilder.newMembershipBuilder()
         .setAuthenticator(
             new GMSAuthenticator(system.getSecurityProperties(), system.getSecurityService(),
                 system.getSecurityLogWriter(), system.getInternalLogWriter()))
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
index ad0c92c..ca3d84a 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
@@ -48,9 +48,7 @@ import org.apache.geode.SystemConnectException;
 import org.apache.geode.SystemFailure;
 import org.apache.geode.annotations.internal.MakeNotStatic;
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.distributed.DistributedSystemDisconnectedException;
-import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.DistributionException;
 import org.apache.geode.distributed.internal.StartupMessage;
@@ -76,7 +74,6 @@ import org.apache.geode.security.GemFireSecurityException;
 
 public class GMSMembership implements Membership {
   private static final Logger logger = Services.getLogger();
-  private final ClusterDistributionManager dm;
 
   /** product version to use for multicast serialization */
   private volatile boolean disableMulticastForRollingUpgrade;
@@ -682,12 +679,11 @@ public class GMSMembership implements Membership {
 
 
   public GMSMembership(MembershipListener listener, MessageListener messageListener,
-      ClusterDistributionManager dm, LifecycleListener lifecycleListener) {
+      LifecycleListener lifecycleListener) {
     this.lifecycleListener = lifecycleListener;
     this.listener = listener;
     this.messageListener = messageListener;
     this.gmsManager = new ManagerImpl();
-    this.dm = dm;
   }
 
   public Manager getGMSManager() {
@@ -853,10 +849,6 @@ public class GMSMembership implements Membership {
 
   /** starts periodic task to perform cleanup chores such as expire surprise members */
   private void startCleanupTimer() {
-    if (dm == null) {
-      return;
-    }
-    DistributedSystem ds = dm.getSystem();
     this.cleanupTimer =
         LoggingExecutors.newScheduledThreadPool("GMSMembership.cleanupTimer", 1, false);
 
@@ -986,8 +978,10 @@ public class GMSMembership implements Membership {
       sender.setMemberData(newID.getMemberData());
       sender.setIsPartial(false);
     } else {
-      // the DM's view also has surprise members, so let's check it as well
-      sender = dm.getCanonicalId(sender);
+      MembershipView currentView = latestView;
+      if (currentView != null) {
+        sender = currentView.getCanonicalID(sender);
+      }
     }
     if (!sender.isPartial()) {
       msg.setSender(sender);
@@ -1749,9 +1743,7 @@ public class GMSMembership implements Membership {
 
   @Override
   public boolean shutdownInProgress() {
-    // Impossible condition (bug36329): make sure that we check DM's
-    // view of shutdown here
-    return shutdownInProgress || (dm != null && dm.shutdownInProgress());
+    return shutdownInProgress;
   }
 
 
@@ -2195,9 +2187,7 @@ public class GMSMembership implements Membership {
 
     @Override
     public boolean shutdownInProgress() {
-      // Impossible condition (bug36329): make sure that we check DM's
-      // view of shutdown here
-      return shutdownInProgress || (dm != null && dm.shutdownInProgress());
+      return shutdownInProgress;
     }
 
     @Override
@@ -2205,10 +2195,6 @@ public class GMSMembership implements Membership {
       return wasReconnectingSystem && !reconnectCompleted;
     }
 
-    @Override
-    public boolean isShutdownStarted() {
-      return shutdownInProgress || (dm != null && dm.isCloseInProgress());
-    }
   }
 
 }
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/MembershipBuilderImpl.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/MembershipBuilderImpl.java
index a87dde1..5628b77 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/MembershipBuilderImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/MembershipBuilderImpl.java
@@ -17,7 +17,6 @@ package org.apache.geode.distributed.internal.membership.gms;
 
 import org.apache.geode.GemFireConfigException;
 import org.apache.geode.SystemConnectException;
-import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.DistributionException;
 import org.apache.geode.distributed.internal.membership.gms.api.Authenticator;
 import org.apache.geode.distributed.internal.membership.gms.api.LifecycleListener;
@@ -39,15 +38,12 @@ public class MembershipBuilderImpl implements MembershipBuilder {
   private MessageListener messageListener;
   private MembershipStatistics statistics;
   private Authenticator authenticator;
-  private ClusterDistributionManager dm;
   private MembershipConfig membershipConfig;
   private DSFIDSerializer serializer;
   private MemberIdentifierFactory memberFactory = new MemberIdentifierFactoryImpl();
   private LifecycleListener lifecycleListener;
 
-  public MembershipBuilderImpl(ClusterDistributionManager dm) {
-    this.dm = dm;
-  }
+  public MembershipBuilderImpl() {}
 
   @Override
   public MembershipBuilder setAuthenticator(Authenticator authenticator) {
@@ -107,7 +103,7 @@ public class MembershipBuilderImpl implements MembershipBuilder {
   @Override
   public Membership create() {
     GMSMembership gmsMembership =
-        new GMSMembership(membershipListener, messageListener, dm, lifecycleListener);
+        new GMSMembership(membershipListener, messageListener, lifecycleListener);
     Services services =
         new Services(gmsMembership.getGMSManager(), statistics, authenticator,
             membershipConfig, serializer, memberFactory, locatorClient);
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MemberIdentifier.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MemberIdentifier.java
index 88857d4..0443476 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MemberIdentifier.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MemberIdentifier.java
@@ -116,4 +116,9 @@ public interface MemberIdentifier extends DataSerializableFixedID {
    * Set the type of node
    */
   void setVmKind(int dmType);
+
+  /**
+   * Set the membership data packed for this identifier
+   */
+  void setMemberData(MemberData data);
 }
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipBuilder.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipBuilder.java
index 477bfdd..aeb14d7 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipBuilder.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipBuilder.java
@@ -15,7 +15,6 @@
 package org.apache.geode.distributed.internal.membership.gms.api;
 
 
-import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.membership.gms.MembershipBuilderImpl;
 import org.apache.geode.distributed.internal.tcpserver.TcpClient;
 import org.apache.geode.internal.serialization.DSFIDSerializer;
@@ -46,7 +45,7 @@ public interface MembershipBuilder {
 
   Membership create();
 
-  static MembershipBuilder newMembershipBuilder(ClusterDistributionManager dm) {
-    return new MembershipBuilderImpl(dm);
+  static MembershipBuilder newMembershipBuilder() {
+    return new MembershipBuilderImpl();
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Manager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Manager.java
index baf8cce..b0bb43d 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Manager.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Manager.java
@@ -57,12 +57,6 @@ public interface Manager extends Service, MessageHandler<Message> {
   boolean shutdownInProgress();
 
   /**
-   * Returns true if a distributed system close is started. And shutdown msg has not sent yet,its in
-   * progress.
-   */
-  boolean isShutdownStarted();
-
-  /**
    * Indicate whether we are attempting a reconnect
    */
   boolean isReconnectingDS();
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
index 224e260..94deae5 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
@@ -878,7 +878,7 @@ public class GMSJoinLeave implements JoinLeave {
 
   boolean isShuttingDown() {
     return services.getCancelCriterion().isCancelInProgress()
-        || services.getManager().shutdownInProgress() || services.getManager().isShutdownStarted();
+        || services.getManager().shutdownInProgress();
   }
 
   boolean prepareView(GMSMembershipView view, List<MemberIdentifier> newMembers)
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipAPIArchUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipAPIArchUnitTest.java
index fe81538..be66073 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipAPIArchUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/api/MembershipAPIArchUnitTest.java
@@ -28,7 +28,6 @@ import com.tngtech.archunit.lang.ArchRule;
 import org.junit.runner.RunWith;
 
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.distributed.internal.membership.MembershipView;
 import org.apache.geode.distributed.internal.membership.gms.MemberDataBuilderImpl;
@@ -63,10 +62,8 @@ public class MembershipAPIArchUnitTest {
               // TODO to be extracted as Interfaces
               .or(type(InternalDistributedMember.class))
               .or(type(MembershipView.class))
-              .or(type(MemberIdentifier.class))
               .or(type(DistributedMember.class))
               .or(type(InternalDistributedMember[].class))
-              .or(type(ClusterDistributionManager.class))
 
               // TODO: This is used by the GMSLocatorAdapter to reach into the locator
               // part of the services