This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-6886
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-6886 by this
push:
new 67fc72b GEODE-6886: Do not region sync if lost member is an empty
accessor
67fc72b is described below
commit 67fc72b75276733d3e41d0edaf9c7953cafe35b1
Author: eshu <e...@pivotal.io>
AuthorDate: Tue Jun 18 10:09:13 2019 -0700
GEODE-6886: Do not region sync if lost member is an empty accessor
* Avoid region sync for empty accessor of persistent replicate region in
correct place.
---
.../distributed/internal/DistributionAdvisor.java | 16 ++++++++
.../geode/internal/cache/DistributedRegion.java | 20 +++-------
.../internal/DistributionAdvisorTest.java | 43 +++++++++++++++++++---
.../internal/cache/DistributedRegionTest.java | 12 +++---
4 files changed, 63 insertions(+), 28 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 7093360..e4716ab 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
@@ -277,6 +277,22 @@ public class DistributionAdvisor {
// through the sync operation. Without associated event information this
could cause the
// retried operation to be mishandled. See GEODE-5505
final long delay = getDelay(dr);
+
+ if (!dr.isInitializedWithWait()) {
+ return;
+ }
+ if (dr.getDataPolicy().withPersistence() && persistentId == null) {
+ // Fix for GEODE-6886 (#46704). The lost member may be an empty accessor
+ // of a persistent replicate region. We don't need to do a
synchronization
+ // in that case, because those members send their writes to a persistent
member.
+ // Only a persistent member can generate the version.
+ if (logger.isDebugEnabled()) {
+ logger.debug(
+ "da.syncForCrashedMember skipping sync because crashed member is
not persistent: {}",
+ id);
+ }
+ return;
+ }
dr.scheduleSynchronizeForLostMember(id, lostVersionID, delay);
if (dr.getConcurrencyChecksEnabled()) {
dr.setRegionSynchronizeScheduled(lostVersionID);
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
index a8022b6..43897a4 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
@@ -1286,18 +1286,7 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
void performSynchronizeForLostMemberTask(InternalDistributedMember member,
VersionSource lostVersionID) {
- waitUntilInitialized();
-
- if (getDataPolicy().withPersistence() && getPersistentID() == null) {
- // Fix for 46704. The lost member may be a replicate
- // or an empty accessor. We don't need to do a synchronization
- // in that case, because those members send their writes to
- // a persistent member.
- if (logger.isDebugEnabled()) {
- logger.debug(
- "da.syncForCrashedMember skipping sync because crashed member is
not persistent: {}",
- member);
- }
+ if (!isInitializedWithWait()) {
return;
}
synchronizeForLostMember(member, lostVersionID);
@@ -1358,10 +1347,10 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
return false;
}
- void waitUntilInitialized() {
+ public boolean isInitializedWithWait() {
while (!isInitialized()) {
if (isDestroyed()) {
- return;
+ return false;
} else {
try {
if (logger.isDebugEnabled()) {
@@ -1370,10 +1359,11 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
}
Thread.sleep(100);
} catch (InterruptedException e) {
- return;
+ return false;
}
}
}
+ return true;
}
/** remove any partial entries received in a failed GII */
diff --git
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionAdvisorTest.java
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionAdvisorTest.java
index a541338..51be3c2 100644
---
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionAdvisorTest.java
+++
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionAdvisorTest.java
@@ -16,6 +16,7 @@ package org.apache.geode.distributed.internal;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -23,6 +24,7 @@ import static org.mockito.Mockito.when;
import org.junit.Before;
import org.junit.Test;
+import org.apache.geode.cache.DataPolicy;
import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.internal.cache.CacheDistributionAdvisor;
import org.apache.geode.internal.cache.DistributedRegion;
@@ -38,6 +40,7 @@ public class DistributionAdvisorTest {
private VersionSource lostVersionID;
private PersistentMemberID persistentMemberID;
private long delay = 100;
+ private DataPolicy dataPolicy;
@Before
public void setup() {
@@ -47,6 +50,13 @@ public class DistributionAdvisorTest {
profile = mock(CacheDistributionAdvisor.CacheProfile.class);
lostVersionID = mock(VersionSource.class);
persistentMemberID = mock(PersistentMemberID.class);
+ dataPolicy = mock(DataPolicy.class);
+
+
when(distributionAdvisor.getRegionForDeltaGII()).thenReturn(distributedRegion);
+ when(distributionAdvisor.getDelay(distributedRegion)).thenReturn(delay);
+ when(distributedRegion.getDataPolicy()).thenReturn(dataPolicy);
+ when(distributedRegion.getConcurrencyChecksEnabled()).thenReturn(true);
+ when(distributedRegion.isInitializedWithWait()).thenReturn(true);
}
@Test
@@ -58,9 +68,7 @@ public class DistributionAdvisorTest {
@Test
public void regionSyncScheduledForLostMember() {
-
when(distributionAdvisor.getRegionForDeltaGII()).thenReturn(distributedRegion);
- when(distributionAdvisor.getDelay(distributedRegion)).thenReturn(delay);
- when(distributedRegion.getConcurrencyChecksEnabled()).thenReturn(true);
+ when(dataPolicy.withPersistence()).thenReturn(false);
doCallRealMethod().when(distributionAdvisor).syncForCrashedMember(member,
profile);
distributionAdvisor.syncForCrashedMember(member, profile);
@@ -71,12 +79,10 @@ public class DistributionAdvisorTest {
@Test
public void regionSyncScheduledForLostPersistentMember() {
-
when(distributionAdvisor.getRegionForDeltaGII()).thenReturn(distributedRegion);
when(distributionAdvisor.getPersistentID((CacheDistributionAdvisor.CacheProfile)
profile))
.thenReturn(persistentMemberID);
when(persistentMemberID.getVersionMember()).thenReturn(lostVersionID);
- when(distributionAdvisor.getDelay(distributedRegion)).thenReturn(delay);
- when(distributedRegion.getConcurrencyChecksEnabled()).thenReturn(true);
+ when(dataPolicy.withPersistence()).thenReturn(true);
doCallRealMethod().when(distributionAdvisor).syncForCrashedMember(member,
profile);
distributionAdvisor.syncForCrashedMember(member, profile);
@@ -84,4 +90,29 @@ public class DistributionAdvisorTest {
verify(distributedRegion).scheduleSynchronizeForLostMember(member,
lostVersionID, delay);
verify(distributedRegion).setRegionSynchronizeScheduled(lostVersionID);
}
+
+ @Test
+ public void regionSyncIsNotScheduledIfRegionIsNotInitialized() {
+ when(distributedRegion.isInitializedWithWait()).thenReturn(false);
+ doCallRealMethod().when(distributionAdvisor).syncForCrashedMember(member,
profile);
+
+ distributionAdvisor.syncForCrashedMember(member, profile);
+
+ verify(distributedRegion,
never()).scheduleSynchronizeForLostMember(member, lostVersionID,
+ delay);
+ verify(distributedRegion,
never()).setRegionSynchronizeScheduled(lostVersionID);
+ }
+
+ @Test
+ public void emptyAccessorOfPersistentRegionDoesNotSynchronizeForLostMember()
{
+ when(dataPolicy.withPersistence()).thenReturn(true);
+ when(distributedRegion.getPersistentID()).thenReturn(null);
+ doCallRealMethod().when(distributionAdvisor).syncForCrashedMember(member,
profile);
+
+ distributionAdvisor.syncForCrashedMember(member, profile);
+
+ verify(distributedRegion,
never()).scheduleSynchronizeForLostMember(member, lostVersionID,
+ delay);
+ verify(distributedRegion,
never()).setRegionSynchronizeScheduled(lostVersionID);
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/DistributedRegionTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/DistributedRegionTest.java
index 23640a4..913b5e3 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/DistributedRegionTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/DistributedRegionTest.java
@@ -157,29 +157,27 @@ public class DistributedRegionTest {
public void
regionSyncInvokedInPerformSynchronizeForLostMemberTaskAfterRegionInitialized() {
DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getDataPolicy()).thenReturn(mock(DataPolicy.class));
+ when(distributedRegion.isInitializedWithWait()).thenReturn(true);
doCallRealMethod().when(distributedRegion).performSynchronizeForLostMemberTask(member,
lostMemberVersionID);
InOrder inOrder = inOrder(distributedRegion);
distributedRegion.performSynchronizeForLostMemberTask(member,
lostMemberVersionID);
- inOrder.verify(distributedRegion).waitUntilInitialized();
+ inOrder.verify(distributedRegion).isInitializedWithWait();
inOrder.verify(distributedRegion).synchronizeForLostMember(member,
lostMemberVersionID);
}
@Test
- public void emptyAccessorOfPersistentRegionDoesNotSynchronizeForLostMember()
{
- DataPolicy dataPolicy = mock(DataPolicy.class);
+ public void
regionSyncNotInvokedInPerformSynchronizeForLostMemberTaskIfRegionNotInitialized()
{
DistributedRegion distributedRegion = mock(DistributedRegion.class);
- when(distributedRegion.getDataPolicy()).thenReturn(dataPolicy);
- when(dataPolicy.withPersistence()).thenReturn(true);
- when(distributedRegion.getPersistentID()).thenReturn(null);
+ when(distributedRegion.getDataPolicy()).thenReturn(mock(DataPolicy.class));
+ when(distributedRegion.isInitializedWithWait()).thenReturn(false);
doCallRealMethod().when(distributedRegion).performSynchronizeForLostMemberTask(member,
lostMemberVersionID);
distributedRegion.performSynchronizeForLostMemberTask(member,
lostMemberVersionID);
- verify(distributedRegion).waitUntilInitialized();
verify(distributedRegion, never()).synchronizeForLostMember(member,
lostMemberVersionID);
}
}
