This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 3ed33a1 GEODE-5307 Hang with servers all in waitForPrimaryMember and
one server in NO_PRIMARY_HOSTING state
3ed33a1 is described below
commit 3ed33a162a11f7f2600f97db4983e820929dd9f3
Author: Bruce Schuchardt <[email protected]>
AuthorDate: Mon Jun 11 13:38:57 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 +-
.../internal/cache/partitioned/RegionAdvisor.java | 10 ++++
.../geode/internal/cache/BucketAdvisorTest.java | 54 ++++++++++++++++++++++
.../java/org/apache/geode/test/dunit/Host.java | 3 +-
7 files changed, 101 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..47f4a82 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) {
+ InternalDistributedMember elector = primaryElector;
+ if (elector != null && regionAdvisor.hasPartitionedRegion(elector)) {
+ // 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.hasPartitionedRegion(newPrimaryElector)) {
+ // 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.hasPartitionedRegion(newPrimaryElector)) {
+ // 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/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
index e2fd72c..c6a9c30 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
@@ -1806,6 +1806,16 @@ public class RegionAdvisor extends
CacheDistributionAdvisor {
}
}
+ /**
+ * return true if the given member has this advisor's partitioned region
+ */
+ public boolean hasPartitionedRegion(InternalDistributedMember profileId) {
+ if (getDistributionManager().getId().equals(profileId)) {
+ return true;
+ }
+ return (getProfile(profileId) != null);
+ }
+
@Override
protected void profileRemoved(Profile profile) {
if (logger.isDebugEnabled()) {
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..9e7ae97 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,22 @@
package org.apache.geode.internal.cache;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+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.mockito.Mockito;
+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 +47,49 @@ 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 missingElectorId = new
InternalDistributedMember("localhost", 123);
+
+ RegionAdvisor regionAdvisor = mock(RegionAdvisor.class);
+ when(regionAdvisor.getPartitionedRegion()).thenReturn(partitionedRegion);
+ // hasPartitionedRegion() is invoked twice - once in
initializePrimaryElector() and then in
+ // volunteerForPrimary(). Returning true first simulates a elector being
+ // there when createBucketAtomically() initiates creation of a bucket.
Returning
+ // false the second time simulates the elector closing its region/cache
before
+ // we get to the point of volunteering for primary
+
when(regionAdvisor.hasPartitionedRegion(Mockito.any(InternalDistributedMember.class)))
+ .thenReturn(true,
+ false);
+
+ 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(missingElectorId);
+ assertEquals(missingElectorId, 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
[email protected].
