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 2018/06/08 23:50:41 UTC
[geode] 01/01: GEODE-5307 Hang with servers all in
waitForPrimaryMember and one server in NO_PRIMARY_HOSTING state
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch feature/GEODE-5307
in repository https://gitbox.apache.org/repos/asf/geode.git
commit fa875084d9e50a7d535ff0910cda664d347fdca8
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Fri Jun 8 16:48:29 2018 -0700
GEODE-5307 Hang with servers all in waitForPrimaryMember and one server in NO_PRIMARY_HOSTING state
Ignore the primaryElector if it is no longer known to the RegionAdvisor.
This means that the elector has somehow gone away - either it crashed,
shut down or destroyed its region.
---
.../distributed/internal/DistributionAdvisor.java | 2 +-
.../apache/geode/internal/cache/BucketAdvisor.java | 50 ++++++++++++-------
.../internal/cache/PRHARedundancyProvider.java | 2 +-
.../internal/cache/PartitionedRegionDataStore.java | 2 +-
.../geode/internal/cache/BucketAdvisorTest.java | 56 ++++++++++++++++++++++
.../java/org/apache/geode/test/dunit/Host.java | 3 +-
6 files changed, 93 insertions(+), 22 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionAdvisor.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionAdvisor.java
index a3be6cf..5ce42ed 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionAdvisor.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionAdvisor.java
@@ -1129,7 +1129,7 @@ public class DistributionAdvisor {
}
/** exchange profiles to initialize this advisor */
- private void exchangeProfiles() {
+ public void exchangeProfiles() {
Assert.assertHoldsLock(this, false); // causes deadlock
Assert.assertHoldsLock(this.initializeLock, true);
new UpdateAttributesProcessor(getAdvisee()).distribute(true);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java
index af386f5..92878af 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java
@@ -493,7 +493,8 @@ public class BucketAdvisor extends CacheDistributionAdvisor {
public void checkForLostPrimaryElector(Profile profile) {
// If the member that went away was in the middle of creating
// the bucket, finish the bucket creation.
- if (this.primaryElector != null && this.primaryElector.equals(profile.getDistributedMember())) {
+ ProfileId elector = this.primaryElector;
+ if (elector != null && elector.equals(profile.getDistributedMember())) {
if (logger.isDebugEnabled()) {
logger.debug(
"Bucket {} lost the member responsible for electing the primary. Finishing bucket creation",
@@ -1002,9 +1003,14 @@ public class BucketAdvisor extends CacheDistributionAdvisor {
* an executor (waiting pool) and returns early.
*/
public void volunteerForPrimary() {
- if (primaryElector != null) {
+ ProfileId elector = primaryElector;
+ if (elector != null && regionAdvisor.getProfile(elector) != null) {
+ // another server will determine the primary node
return;
}
+
+ primaryElector = null;
+
initializationGate();
synchronized (this) {
@@ -1012,13 +1018,19 @@ public class BucketAdvisor extends CacheDistributionAdvisor {
// only one thread should be attempting to volunteer at one time
return;
}
+
if (this.volunteeringDelegate == null) {
- this.volunteeringDelegate = new VolunteeringDelegate();
+ setVolunteeringDelegate(new VolunteeringDelegate());
}
this.volunteeringDelegate.volunteerForPrimary();
+
}
}
+ protected void setVolunteeringDelegate(VolunteeringDelegate delegate) {
+ this.volunteeringDelegate = delegate;
+ }
+
/**
* Makes this <code>BucketAdvisor</code> become the primary if it is already a secondary.
*
@@ -1520,29 +1532,35 @@ public class BucketAdvisor extends CacheDistributionAdvisor {
}
}
- public void clearPrimaryElector() {
- synchronized (this) {
- primaryElector = null;
- }
+ public synchronized void clearPrimaryElector() {
+ primaryElector = null;
}
- public void setPrimaryElector(InternalDistributedMember newPrimaryElector) {
- synchronized (this) {
- // Only set the new primary elector if we have not yet seen
- // a primary for this bucket.
- if (primaryElector != null) {
+ public synchronized void setPrimaryElector(InternalDistributedMember newPrimaryElector) {
+ // Only set the new primary elector if we have not yet seen
+ // a primary for this bucket.
+ if (this.primaryElector != null) {
+ if (newPrimaryElector != null && regionAdvisor.getProfile(newPrimaryElector) == null) {
+ // no longer a participant - don't use it
+ this.primaryElector = null;
+ } else {
this.primaryElector = newPrimaryElector;
}
}
}
- public synchronized void initializePrimaryElector(InternalDistributedMember primaryElector) {
+ public synchronized void initializePrimaryElector(InternalDistributedMember newPrimaryElector) {
// For child buckets, we want the parent bucket to take care'
// of finishing an incomplete bucket creation, so only set the elector for
// the leader region.
if (parentAdvisor == null) {
- this.primaryElector = primaryElector;
+ if (newPrimaryElector != null && regionAdvisor.getProfile(newPrimaryElector) == null) {
+ // no longer a participant - don't use it
+ this.primaryElector = null;
+ } else {
+ this.primaryElector = newPrimaryElector;
+ }
}
}
@@ -1605,9 +1623,7 @@ public class BucketAdvisor extends CacheDistributionAdvisor {
* if (needToNotPrimarySelf) { notPrimary(getAdvisee().getDistributionManager().getId()); }
*/
if (needToVolunteerForPrimary) {
- if (this.primaryElector == null) {
- volunteerForPrimary();
- }
+ volunteerForPrimary();
}
sendProfileUpdate();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
index f098d72..08ef757 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
@@ -571,7 +571,7 @@ public class PRHARedundancyProvider {
observer = new BucketMembershipObserver(toCreate).beginMonitoring();
boolean loggedInsufficentStores = false; // track if insufficient data stores have been
- // detected
+ // detected
for (;;) {
this.prRegion.checkReadiness();
if (this.prRegion.getCache().isCacheAtShutdownAll()) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
index 8f308ca..9048851 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
@@ -2902,7 +2902,7 @@ public class PartitionedRegionDataStore implements HasCachePerfStats {
// Assert.assertTrue(nList.contains(partitionedRegion.getNode().getMemberId()) ,
// " grab returned false and b2n does not contains this member.");
} else {
- // try grabbing bucekts for all the PR which are colocated with it
+ // try grabbing buckets for all the PR which are colocated with it
List colocatedWithList = ColocationHelper.getColocatedChildRegions(partitionedRegion);
Iterator itr = colocatedWithList.iterator();
while (itr.hasNext()) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketAdvisorTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketAdvisorTest.java
index 7d2946b..4f86d0f 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketAdvisorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketAdvisorTest.java
@@ -15,13 +15,23 @@
package org.apache.geode.internal.cache;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import org.apache.geode.distributed.internal.DistributionAdvisor;
+import org.apache.geode.distributed.internal.DistributionManager;
import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.Bucket;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
import org.apache.geode.test.junit.categories.UnitTest;
@Category(UnitTest.class)
@@ -38,4 +48,50 @@ public class BucketAdvisorTest {
assertThat(mockBucketAdvisor.basicGetPrimaryMember()).isEqualTo(mockInternalDistributedMember);
assertThat(mockBucketAdvisor.getBucketRedundancy()).isEqualTo(1);
}
+
+ @Test
+ public void volunteerForPrimaryIgnoresMissingPrimaryElector() {
+ DistributionManager distributionManager = mock(DistributionManager.class);
+ when(distributionManager.getId()).thenReturn(new InternalDistributedMember("localhost", 321));
+
+ Bucket bucket = mock(Bucket.class);
+ when(bucket.isHosting()).thenReturn(true);
+ when(bucket.isPrimary()).thenReturn(false);
+ when(bucket.getDistributionManager()).thenReturn(distributionManager);
+
+ PartitionedRegion partitionedRegion = mock(PartitionedRegion.class);
+ when(partitionedRegion.getRedundantCopies()).thenReturn(0);
+ when(partitionedRegion.getPartitionAttributes()).thenReturn(new PartitionAttributesImpl());
+ when(partitionedRegion.getRedundancyTracker())
+ .thenReturn(mock(PartitionedRegionRedundancyTracker.class));
+
+ InternalDistributedMember memberId = new InternalDistributedMember("localhost", 123);
+ DistributionAdvisor.Profile profile = new BucketAdvisor.BucketProfile(
+ memberId, 1, bucket);
+
+ RegionAdvisor regionAdvisor = mock(RegionAdvisor.class);
+ when(regionAdvisor.getPartitionedRegion()).thenReturn(partitionedRegion);
+ // getProfile() is invoked twice - once in initializePrimaryElector() and then in
+ // volunteerForPrimary(). Returning a profile first simulates a elector being
+ // there when createBucketAtomically() initiates creation of a bucket. Returning
+ // null the second time simulates the elector closing its region/cache before
+ // we get to the point of volunteering for primary
+ when(regionAdvisor.getProfile(isA(DistributionAdvisor.ProfileId.class))).thenReturn(profile,
+ null);
+
+ BucketAdvisor advisor = BucketAdvisor.createBucketAdvisor(bucket, regionAdvisor);
+ BucketAdvisor advisorSpy = spy(advisor);
+ doCallRealMethod().when(advisorSpy).exchangeProfiles();
+ doCallRealMethod().when(advisorSpy).volunteerForPrimary();
+ doReturn(true).when(advisorSpy).initializationGate();
+ doReturn(true).when(advisorSpy).isHosting();
+
+ BucketAdvisor.VolunteeringDelegate volunteeringDelegate =
+ mock(BucketAdvisor.VolunteeringDelegate.class);
+ advisorSpy.setVolunteeringDelegate(volunteeringDelegate);
+ advisorSpy.initializePrimaryElector(memberId);
+ assertEquals(memberId, advisorSpy.getPrimaryElector());
+ advisorSpy.volunteerForPrimary();
+ verify(volunteeringDelegate).volunteerForPrimary();
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/Host.java b/geode-core/src/test/java/org/apache/geode/test/dunit/Host.java
index e3e93c9..18f17c3 100755
--- a/geode-core/src/test/java/org/apache/geode/test/dunit/Host.java
+++ b/geode-core/src/test/java/org/apache/geode/test/dunit/Host.java
@@ -30,9 +30,7 @@ import org.apache.geode.test.dunit.standalone.VersionManager;
* RMI registry is only started on the host on which Hydra's Master VM runs. RMI registries may be
* started on other hosts via additional Hydra configuration.
*
- * @deprecated Please use similar static APIs on {@link VM} instead.
*/
-@Deprecated
@SuppressWarnings("serial")
public abstract class Host implements Serializable {
@@ -131,6 +129,7 @@ public abstract class Host implements Serializable {
* @param n A zero-based identifier of the VM
*
* @throws IllegalArgumentException {@code n} is more than the number of VMs
+ * @deprecated use the static methods in VM instead
*/
public VM getVM(int n) {
int size = vms.size();
--
To stop receiving notification emails like this one, please contact
bschuchardt@apache.org.