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 <[email protected]> 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
