geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject [geode] 01/01: GEODE-5307 Hang with servers all in waitForPrimaryMember and one server in NO_PRIMARY_HOSTING state
Date Fri, 08 Jun 2018 23:50:41 GMT
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 <bschuchardt@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.

Mime
View raw message