This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.15 by this push:
new 5936977165 GEODE-10286: handle CancelException in
PersistenceAdvisor.close (#7677)
5936977165 is described below
commit 5936977165efa969368d35400e8f521d7bf0add9
Author: Jinmei Liao <jil...@pivotal.io>
AuthorDate: Mon May 16 15:58:28 2022 -0700
GEODE-10286: handle CancelException in PersistenceAdvisor.close (#7677)
(cherry picked from commit e1860051f978cbd02d2bccd648175a7b79252f75)
---
.../geode/internal/cache/DistributedRegion.java | 5 ++--
.../cache/persistence/PersistenceAdvisorImpl.java | 19 ++++++++----
.../internal/cache/DistributedRegionTest.java | 34 +++++-----------------
.../persistence/PersistenceAdvisorImplTest.java | 20 ++++++++++---
4 files changed, 40 insertions(+), 38 deletions(-)
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 c247839608..faadd1ad0e 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
@@ -2604,6 +2604,7 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
ex);
}
}
+
waitForCurrentOperations();
}
@@ -2615,9 +2616,9 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
if (!cache.forcedDisconnect() && flushOnClose
&& getDistributionManager().getDistribution() != null
&& getDistributionManager().getDistribution().isConnected()) {
- getDistributionAdvisor().forceNewMembershipVersion();
+ distAdvisor.forceNewMembershipVersion();
try {
- getDistributionAdvisor().waitForCurrentOperations();
+ distAdvisor.waitForCurrentOperations();
} catch (Exception e) {
// log this but try to close the region so that listeners are invoked
logger.warn(String.format("%s: error closing region %s", this,
getFullPath()), e);
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
index b5e7f4f2b6..174e4e042c 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
@@ -30,6 +30,7 @@ import java.util.Set;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.TestOnly;
+import org.apache.geode.CancelException;
import org.apache.geode.annotations.Immutable;
import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.annotations.internal.MutableForTesting;
@@ -1161,11 +1162,19 @@ public class PersistenceAdvisorImpl implements
InternalPersistenceAdvisor {
@Override
public void close() {
- isClosed = true;
- persistentMemberManager.removeRevocationListener(profileChangeListener);
-
cacheDistributionAdvisor.removeProfileChangeListener(profileChangeListener);
- persistentStateListeners = Collections.emptySet();
- releaseTieLock();
+ try {
+ synchronized (this) {
+ isClosed = true;
+
persistentMemberManager.removeRevocationListener(profileChangeListener);
+
cacheDistributionAdvisor.removeProfileChangeListener(profileChangeListener);
+ persistentStateListeners = Collections.emptySet();
+ releaseTieLock();
+ }
+ } catch (CancelException e) {
+ logger.debug("persistence advisor close abridged due to shutdown", e);
+ } catch (Exception ex) {
+ logger.warn("persistence advisor close has failed.", ex);
+ }
}
/**
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 2de238e015..e90ca759e6 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
@@ -30,8 +30,8 @@ import static org.mockito.Mockito.when;
import java.util.Collections;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.InOrder;
import org.apache.geode.cache.DataPolicy;
@@ -53,8 +53,9 @@ public class DistributedRegionTest {
private InternalDistributedMember member;
private EntryEventImpl event;
private EventTracker eventTracker;
+ private DistributedRegion distributedRegion;
- @Before
+ @BeforeEach
@SuppressWarnings("unchecked")
public void setup() {
vector = mock(RegionVersionVector.class);
@@ -63,18 +64,18 @@ public class DistributedRegionTest {
member = mock(InternalDistributedMember.class);
event = mock(EntryEventImpl.class);
eventTracker = mock(EventTracker.class);
+ distributedRegion = mock(DistributedRegion.class);
}
@Test
public void shouldBeMockable() throws Exception {
- DistributedRegion mockDistributedRegion = mock(DistributedRegion.class);
EntryEventImpl mockEntryEventImpl = mock(EntryEventImpl.class);
Object returnValue = new Object();
- when(mockDistributedRegion.validatedDestroy(any(), eq(mockEntryEventImpl)))
+ when(distributedRegion.validatedDestroy(any(), eq(mockEntryEventImpl)))
.thenReturn(returnValue);
- assertThat(mockDistributedRegion.validatedDestroy(new Object(),
mockEntryEventImpl))
+ assertThat(distributedRegion.validatedDestroy(new Object(),
mockEntryEventImpl))
.isSameAs(returnValue);
}
@@ -96,7 +97,6 @@ public class DistributedRegionTest {
@Test
public void
cleanUpAfterFailedInitialImageDoesNotCloseEntriesIfIsPersistentRegionAndRecoveredFromDisk()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
DiskRegion diskRegion = mock(DiskRegion.class);
doCallRealMethod().when(distributedRegion).cleanUpAfterFailedGII(true);
@@ -111,7 +111,6 @@ public class DistributedRegionTest {
@Test
public void lockHeldWhenRegionIsNotInitialized() {
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
when(distributedRegion.isInitialized()).thenReturn(false);
@@ -121,7 +120,6 @@ public class DistributedRegionTest {
@Test
public void lockNotHeldWhenRegionIsInitialized() {
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
when(distributedRegion.isInitialized()).thenReturn(true);
@@ -131,7 +129,6 @@ public class DistributedRegionTest {
@Test
public void
versionHolderInvokesSetRegionSynchronizeScheduledIfVectorContainsLostMemberID()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getVersionVector()).thenReturn(vector);
when(vector.getHolderForMember(lostMemberVersionID)).thenReturn(holder);
doCallRealMethod().when(distributedRegion).setRegionSynchronizeScheduled(lostMemberVersionID);
@@ -143,7 +140,6 @@ public class DistributedRegionTest {
@Test
public void
versionHolderInvokesSetRegionSynchronizeScheduledOrDoneIfNotIfVectorContainsLostMemberID()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getVersionVector()).thenReturn(vector);
when(vector.getHolderForMember(lostMemberVersionID)).thenReturn(holder);
doCallRealMethod().when(distributedRegion)
@@ -158,7 +154,6 @@ public class DistributedRegionTest {
@Test
public void
setRegionSynchronizedWithIfNotScheduledReturnsFalseIfVectorDoesNotContainLostMemberID()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getVersionVector()).thenReturn(vector);
when(vector.getHolderForMember(lostMemberVersionID)).thenReturn(holder);
@@ -170,7 +165,6 @@ public class DistributedRegionTest {
@Test
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,
@@ -185,7 +179,6 @@ public class DistributedRegionTest {
@Test
public void
regionSyncNotInvokedInPerformSynchronizeForLostMemberTaskIfRegionNotInitialized()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getDataPolicy()).thenReturn(mock(DataPolicy.class));
when(distributedRegion.isInitializedWithWait()).thenReturn(false);
doCallRealMethod().when(distributedRegion).performSynchronizeForLostMemberTask(member,
@@ -201,7 +194,6 @@ public class DistributedRegionTest {
InternalCache internalCache = mock(InternalCache.class);
when(internalCache.getAllGatewaySenders())
.thenReturn(Collections.singleton(mock(GatewaySender.class)));
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getCache()).thenReturn(internalCache);
doCallRealMethod().when(distributedRegion).validateAsynchronousEventDispatcher(anyString());
@@ -216,7 +208,6 @@ public class DistributedRegionTest {
when(serialSender.getId()).thenReturn(senderId);
InternalCache internalCache = mock(InternalCache.class);
when(internalCache.getAllGatewaySenders()).thenReturn(Collections.singleton(serialSender));
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getCache()).thenReturn(internalCache);
doCallRealMethod().when(distributedRegion).validateAsynchronousEventDispatcher(anyString());
@@ -234,7 +225,6 @@ public class DistributedRegionTest {
InternalCache internalCache = mock(InternalCache.class);
when(internalCache.getAllGatewaySenders())
.thenReturn(Collections.singleton(parallelAsyncEventQueue));
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getCache()).thenReturn(internalCache);
when(distributedRegion.getFullPath()).thenReturn(regionPath);
doCallRealMethod().when(distributedRegion).validateAsynchronousEventDispatcher(anyString());
@@ -256,7 +246,6 @@ public class DistributedRegionTest {
InternalCache internalCache = mock(InternalCache.class);
when(internalCache.getAllGatewaySenders())
.thenReturn(Collections.singleton(parallelGatewaySender));
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
when(distributedRegion.getCache()).thenReturn(internalCache);
when(distributedRegion.getFullPath()).thenReturn(regionPath);
doCallRealMethod().when(distributedRegion).validateAsynchronousEventDispatcher(anyString());
@@ -269,7 +258,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndVersionTagIsSet()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(true);
@@ -282,7 +270,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndConcurrencyChecksNotEnabled()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(true);
@@ -296,7 +283,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndEventIdIsNotSet()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(true);
@@ -311,7 +297,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventWillFindAndSetVersionTagIfFoundInEventTrackerButValidTagNotSet() {
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(true);
@@ -326,7 +311,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundEventInEventTrackerAndNotADuplicateEvent()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(false);
@@ -338,7 +322,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndConcurrencyChecksNotEnabled()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(false);
@@ -352,7 +335,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndVersionTagIsSet()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(false);
@@ -367,7 +349,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndNoEventId() {
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(false);
@@ -382,7 +363,6 @@ public class DistributedRegionTest {
@Test
public void
hasSeenEventWillFindAndSetVersionTagIfNotFoundInEventTrackerAndIsPossibleDuplicateWithConcurrencyChecksEnabled()
{
- DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).hasSeenEvent(event);
when(distributedRegion.getEventTracker()).thenReturn(eventTracker);
when(eventTracker.hasSeenEvent(event)).thenReturn(false);
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImplTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImplTest.java
index 9c495db96b..3210940eef 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImplTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImplTest.java
@@ -20,9 +20,11 @@ import static java.util.Collections.singleton;
import static java.util.Collections.singletonMap;
import static org.apache.geode.cache.Region.SEPARATOR;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
@@ -35,10 +37,11 @@ import java.util.Set;
import java.util.TreeSet;
import java.util.UUID;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
+import org.apache.geode.ForcedDisconnectException;
import org.apache.geode.distributed.DistributedLockService;
import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.internal.cache.CacheDistributionAdvisor;
@@ -69,13 +72,15 @@ public class PersistenceAdvisorImplTest {
private PersistentStateQueryResults persistentStateQueryResults;
private PersistenceAdvisorImpl persistenceAdvisorImpl;
+ private PersistentMemberManager persistentMemberManager;
private int diskStoreIDIndex = 92837487; // some random number
- @Before
+ @BeforeEach
public void setUp() throws Exception {
cacheDistributionAdvisor = mock(CacheDistributionAdvisor.class);
persistentMemberView = mock(DiskRegion.class);
+ persistentMemberManager = mock(PersistentMemberManager.class);
PersistentStateQueryMessageSenderFactory queryMessageSenderFactory =
mock(PersistentStateQueryMessageSenderFactory.class);
PersistentStateQueryMessage queryMessage =
@@ -91,10 +96,17 @@ public class PersistenceAdvisorImplTest {
persistenceAdvisorImpl =
new PersistenceAdvisorImpl(cacheDistributionAdvisor, null,
persistentMemberView, null, null,
- null, mock(StartupStatus.class), mock(Transformer.class),
+ persistentMemberManager, mock(StartupStatus.class),
mock(Transformer.class),
mock(CollectionTransformer.class), queryMessageSenderFactory);
}
+ @Test
+ void closeDoesNotThrowException() {
+ persistenceAdvisorImpl = spy(persistenceAdvisorImpl);
+ doThrow(new
ForcedDisconnectException("test")).when(persistenceAdvisorImpl).releaseTieLock();
+ assertThatCode(() ->
persistenceAdvisorImpl.close()).doesNotThrowAnyException();
+ }
+
/**
* GEODE-5402: This test creates a scenario where a member has two versions
(based on timeStamp)
* of another member. The call to getMembersToWaitFor should return that we
wait for neither of
