Skip to content
Permalink
Browse files
GEODE-10286: handle CancelException in PersistenceAdvisor.close (#7677)
  • Loading branch information
jinmeiliao committed May 16, 2022
1 parent e835c8c commit e1860051f978cbd02d2bccd648175a7b79252f75
Showing 4 changed files with 40 additions and 38 deletions.
@@ -2604,6 +2604,7 @@ protected void distributedRegionCleanup(RegionEventImpl event) {
ex);
}
}

waitForCurrentOperations();
}

@@ -2615,9 +2616,9 @@ private void waitForCurrentOperations() {
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);
@@ -30,6 +30,7 @@
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 void checkInterruptedByShutdownAll() {

@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);
}
}

/**
@@ -30,8 +30,8 @@

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 void setup() {
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 void cleanUpAfterFailedInitialImageHoldsLockForClear() {

@Test
public void cleanUpAfterFailedInitialImageDoesNotCloseEntriesIfIsPersistentRegionAndRecoveredFromDisk() {
DistributedRegion distributedRegion = mock(DistributedRegion.class);
DiskRegion diskRegion = mock(DiskRegion.class);

doCallRealMethod().when(distributedRegion).cleanUpAfterFailedGII(true);
@@ -111,7 +111,6 @@ public void cleanUpAfterFailedInitialImageDoesNotCloseEntriesIfIsPersistentRegio

@Test
public void lockHeldWhenRegionIsNotInitialized() {
DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
when(distributedRegion.isInitialized()).thenReturn(false);

@@ -121,7 +120,6 @@ public void lockHeldWhenRegionIsNotInitialized() {

@Test
public void lockNotHeldWhenRegionIsInitialized() {
DistributedRegion distributedRegion = mock(DistributedRegion.class);
doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
when(distributedRegion.isInitialized()).thenReturn(true);

@@ -131,7 +129,6 @@ public void lockNotHeldWhenRegionIsInitialized() {

@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 void versionHolderInvokesSetRegionSynchronizeScheduledIfVectorContainsLos

@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 void versionHolderInvokesSetRegionSynchronizeScheduledOrDoneIfNotIfVector

@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 void setRegionSynchronizedWithIfNotScheduledReturnsFalseIfVectorDoesNotCo

@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 void regionSyncInvokedInPerformSynchronizeForLostMemberTaskAfterRegionIni

@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 void validateAsynchronousEventDispatcherShouldDoNothingWhenDispatcherIdCa
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 void validateAsynchronousEventDispatcherShouldDoNothingWhenFoundDispatche
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 void validateAsynchronousEventDispatcherShouldThrowExceptionWhenDispatche
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 void validateAsynchronousEventDispatcherShouldThrowExceptionWhenDispatche
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 void validateAsynchronousEventDispatcherShouldThrowExceptionWhenDispatche

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndVersi

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndConcu

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfFoundInEventTrackerAndEvent

@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 void hasSeenEventWillFindAndSetVersionTagIfFoundInEventTrackerButValidTag

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundEventInEventTracker

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndCo

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndVe

@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 void hasSeenEventDoesNotFindAndSetVersionTagIfNotFoundInEventTrackerAndNo

@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);
@@ -20,9 +20,11 @@
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.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 void setUp() throws Exception {

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

0 comments on commit e186005

Please sign in to comment.