This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-5748
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-5748 by this
push:
new 45baa7e GEODE-5748: Hold a write lock during cleanUpAfterFailedGII
45baa7e is described below
commit 45baa7e8c0e6dc7616858700a0193579dae50ae6
Author: eshu <[email protected]>
AuthorDate: Thu Sep 20 16:54:04 2018 -0700
GEODE-5748: Hold a write lock during cleanUpAfterFailedGII
* Hold write lock when cleanUpAfterFailedGII.
* Hold read lock when cache operation is performed on the farside if
region is not yet initialized.
* Also hold read lock when transaction is performed on the farside if
region not initialized yet.
---
.../geode/internal/cache/AbstractRegionMap.java | 31 ++++-
.../apache/geode/internal/cache/BucketRegion.java | 5 +
.../geode/internal/cache/DistributedRegion.java | 46 ++++++-
.../geode/internal/cache/InternalRegion.java | 6 +
.../geode/internal/cache/map/RegionMapDestroy.java | 4 +
.../geode/internal/cache/map/RegionMapPut.java | 4 +
.../internal/cache/AbstractRegionMapTest.java | 143 ++++++++++++++++++++-
.../internal/cache/DistributedRegionTest.java | 32 +++--
.../internal/cache/map/RegionMapDestroyTest.java | 36 ++++++
.../geode/internal/cache/map/RegionMapPutTest.java | 36 ++++++
10 files changed, 318 insertions(+), 25 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
index bc1ca9b..565d2ed 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
@@ -1038,6 +1038,7 @@ public abstract class AbstractRegionMap
final boolean hasRemoteOrigin =
!txId.getMemberId().equals(owner.getMyId());
boolean callbackEventAddedToPending = false;
IndexManager oqlIndexManager = owner.getIndexManager();
+ boolean locked = owner.lockWhenRegionIsInitializing();
try {
RegionEntry re = getEntry(key);
if (re != null) {
@@ -1291,6 +1292,10 @@ public abstract class AbstractRegionMap
} catch (DiskAccessException dae) {
owner.handleDiskAccessException(dae);
throw dae;
+ } finally {
+ if (locked) {
+ owner.unlockWhenRegionIsInitializing();
+ }
}
}
@@ -1337,6 +1342,7 @@ public abstract class AbstractRegionMap
boolean clearOccured = false;
DiskRegion dr = owner.getDiskRegion();
boolean ownerIsInitialized = owner.isInitialized();
+ boolean locked = false;
try {
// Fix for Bug #44431. We do NOT want to update the region and wait
// later for index INIT as region.clear() can cause inconsistency if
@@ -1346,6 +1352,7 @@ public abstract class AbstractRegionMap
oqlIndexManager.waitForIndexInit();
}
lockForCacheModification(owner, event);
+ locked = owner.lockWhenRegionIsInitializing();
try {
try {
if (forceNewEntry || forceCallbacks) {
@@ -1710,6 +1717,9 @@ public abstract class AbstractRegionMap
}
} finally {
releaseCacheModificationLock(owner, event);
+ if (locked) {
+ owner.unlockWhenRegionIsInitializing();
+ }
}
}
@@ -1770,6 +1780,7 @@ public abstract class AbstractRegionMap
}
lockForCacheModification(owner, event);
+ boolean locked = owner.lockWhenRegionIsInitializing();
try {
RegionEntry re = getEntry(event.getKey());
@@ -1802,6 +1813,9 @@ public abstract class AbstractRegionMap
throw dae;
} finally {
releaseCacheModificationLock(owner, event);
+ if (locked) {
+ owner.unlockWhenRegionIsInitializing();
+ }
if (dr != null) {
dr.removeClearCountReference();
}
@@ -1831,6 +1845,7 @@ public abstract class AbstractRegionMap
if (oqlIndexManager != null) {
oqlIndexManager.waitForIndexInit();
}
+ boolean locked = owner.lockWhenRegionIsInitializing();
try {
if (forceNewEntry) {
boolean opCompleted = false;
@@ -2042,6 +2057,9 @@ public abstract class AbstractRegionMap
if (oqlIndexManager != null) {
oqlIndexManager.countDownIndexUpdaters();
}
+ if (locked) {
+ owner.unlockWhenRegionIsInitializing();
+ }
}
}
@@ -2112,9 +2130,16 @@ public abstract class AbstractRegionMap
callbackEvent.makeSerializedNewValue();
txHandleWANEvent(owner, callbackEvent, txEntryState);
}
- RegionMapCommitPut commitPut = new RegionMapCommitPut(this, owner,
callbackEvent, putOp,
- didDestroy, txId, txEvent, pendingCallbacks, txEntryState);
- commitPut.put();
+ boolean locked = owner.lockWhenRegionIsInitializing();
+ try {
+ RegionMapCommitPut commitPut = new RegionMapCommitPut(this, owner,
callbackEvent, putOp,
+ didDestroy, txId, txEvent, pendingCallbacks, txEntryState);
+ commitPut.put();
+ } finally {
+ if (locked) {
+ owner.unlockWhenRegionIsInitializing();
+ }
+ }
}
private void txHandleWANEvent(final LocalRegion owner, EntryEventImpl
callbackEvent,
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
index 0fb745d..4dbed7c 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
@@ -1341,8 +1341,10 @@ public class BucketRegion extends DistributedRegion
implements Bucket {
InternalRegion internalRegion = event.getRegion();
AbstractRegionMap arm = ((AbstractRegionMap)
internalRegion.getRegionMap());
+ boolean locked = false;
try {
arm.lockForCacheModification(internalRegion, event);
+ locked = internalRegion.lockWhenRegionIsInitializing();
beginLocalWrite(event);
try {
if (!hasSeenEvent(event)) {
@@ -1366,6 +1368,9 @@ public class BucketRegion extends DistributedRegion
implements Bucket {
}
} finally {
arm.releaseCacheModificationLock(event.getRegion(), event);
+ if (locked) {
+ internalRegion.unlockWhenRegionIsInitializing();
+ }
}
}
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 488b1fa..c6c8a43 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
@@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.logging.log4j.Logger;
@@ -181,6 +182,7 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
* Lock to prevent multiple threads on this member from performing a clear
at the same time.
*/
private final Object clearLock = new Object();
+ private final ReentrantReadWriteLock failedGIIClearLock = new
ReentrantReadWriteLock(true);
private static final AtomicBoolean loggedNetworkPartitionWarning = new
AtomicBoolean(false);
@@ -1332,10 +1334,7 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
}
if (!getRegionMap().isEmpty()) {
- RegionVersionVector rvv = getVersionVector();
- if (rvv != null) {
- rvv.lockForClear(getFullPath(), getDistributionManager(), getMyId());
- }
+ lockFailedGIIClearWriteLock();
try {
closeEntries();
if (getDiskRegion() != null) {
@@ -1355,13 +1354,41 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
}
}
} finally {
- if (rvv != null) {
- rvv.unlockForClear(getMyId());
- }
+ unlockFailedGIIClearWriteLock();
}
}
}
+ void lockFailedGIIClearWriteLock() {
+ failedGIIClearLock.writeLock().lock();
+ }
+
+ void unlockFailedGIIClearWriteLock() {
+ failedGIIClearLock.writeLock().unlock();
+ }
+
+ void lockFailedGIIClearReadLock() {
+ failedGIIClearLock.readLock().lock();
+ }
+
+ void unlockFailedGIIClearReadLock() {
+ failedGIIClearLock.readLock().unlock();
+ }
+
+ @Override
+ public boolean lockWhenRegionIsInitializing() {
+ if (!isInitialized()) {
+ lockFailedGIIClearReadLock();
+ return true;
+ }
+ return false;
+ }
+
+ @Override
+ public void unlockWhenRegionIsInitializing() {
+ unlockFailedGIIClearReadLock();
+ }
+
private void initMembershipRoles() {
synchronized (this.advisorListener) {
// hold sync to prevent listener from changing initial members
@@ -1853,8 +1880,10 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
void basicUpdateEntryVersion(EntryEventImpl event) throws
EntryNotFoundException {
InternalRegion internalRegion = event.getRegion();
AbstractRegionMap regionMap = (AbstractRegionMap)
internalRegion.getRegionMap();
+ boolean locked = false;
try {
regionMap.lockForCacheModification(internalRegion, event);
+ locked = internalRegion.lockWhenRegionIsInitializing();
try {
if (!hasSeenEvent(event)) {
super.basicUpdateEntryVersion(event);
@@ -1866,6 +1895,9 @@ public class DistributedRegion extends LocalRegion
implements InternalDistribute
}
} finally {
regionMap.releaseCacheModificationLock(internalRegion, event);
+ if (locked) {
+ internalRegion.unlockWhenRegionIsInitializing();
+ }
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
index d21d130..caeb9f9 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
@@ -411,4 +411,10 @@ public interface InternalRegion extends Region,
HasCachePerfStats, RegionEntryCo
MemoryThresholdInfo getAtomicThresholdInfo();
InternalCache getInternalCache();
+
+ default boolean lockWhenRegionIsInitializing() {
+ return false;
+ };
+
+ default void unlockWhenRegionIsInitializing() {};
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
index 39b65de..5a49fef 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
@@ -105,6 +105,7 @@ public class RegionMapDestroy {
}
cacheModificationLock.lockForCacheModification(internalRegion, event);
+ boolean locked = internalRegion.lockWhenRegionIsInitializing();
try {
while (retry) {
@@ -173,6 +174,9 @@ public class RegionMapDestroy {
} finally {
cacheModificationLock.releaseCacheModificationLock(internalRegion,
event);
+ if (locked) {
+ internalRegion.unlockWhenRegionIsInitializing();
+ }
}
return false;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
index 50dc4fa..813f717 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
@@ -146,10 +146,14 @@ public class RegionMapPut extends AbstractRegionMapPut {
@Override
protected void runWhileLockedForCacheModification(Runnable r) {
cacheModificationLock.lockForCacheModification(getOwner(), getEvent());
+ boolean locked = getOwner().lockWhenRegionIsInitializing();
try {
r.run();
} finally {
cacheModificationLock.releaseCacheModificationLock(getOwner(),
getEvent());
+ if (locked) {
+ getOwner().unlockWhenRegionIsInitializing();
+ }
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
index 390a3b3..212a1b8 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
@@ -25,6 +25,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -1186,15 +1187,22 @@ public class AbstractRegionMapTest {
protected TestableAbstractRegionMap(boolean withConcurrencyChecks,
ConcurrentMapWithReusableEntries map, RegionEntryFactory factory) {
- this(withConcurrencyChecks, map, factory, null);
+ this(withConcurrencyChecks, false, map, factory, null);
}
protected TestableAbstractRegionMap(boolean withConcurrencyChecks,
ConcurrentMapWithReusableEntries map, RegionEntryFactory factory,
RegionEntry regionEntryForGetEntry) {
+ this(withConcurrencyChecks, false, map, factory, regionEntryForGetEntry);
+ }
+
+ protected TestableAbstractRegionMap(boolean withConcurrencyChecks, boolean
isDistributedRegion,
+ ConcurrentMapWithReusableEntries map, RegionEntryFactory factory,
+ RegionEntry regionEntryForGetEntry) {
super(null);
this.regionEntryForGetEntry = regionEntryForGetEntry;
- LocalRegion owner = mock(LocalRegion.class);
+ LocalRegion owner = isDistributedRegion ? mock(DistributedRegion.class,
RETURNS_DEEP_STUBS)
+ : mock(LocalRegion.class);
CachePerfStats cachePerfStats = mock(CachePerfStats.class);
when(owner.getCachePerfStats()).thenReturn(cachePerfStats);
when(owner.getConcurrencyChecksEnabled()).thenReturn(withConcurrencyChecks);
@@ -1268,19 +1276,30 @@ public class AbstractRegionMapTest {
*/
private static class TxTestableAbstractRegionMap extends AbstractRegionMap {
- protected TxTestableAbstractRegionMap() {
+ protected TxTestableAbstractRegionMap(boolean isInitialized) {
super(null);
- LocalRegion owner = mock(LocalRegion.class);
+ InternalRegion owner;
+ if (isInitialized) {
+ owner = mock(LocalRegion.class);
+ when(owner.isInitialized()).thenReturn(true);
+ } else {
+ owner = mock(DistributedRegion.class);
+ when(owner.isInitialized()).thenReturn(false);
+ }
KeyInfo keyInfo = mock(KeyInfo.class);
when(keyInfo.getKey()).thenReturn(KEY);
when(owner.getKeyInfo(eq(KEY), any(), any())).thenReturn(keyInfo);
when(owner.getMyId()).thenReturn(mock(InternalDistributedMember.class));
when(owner.getCache()).thenReturn(mock(InternalCache.class));
when(owner.isAllEvents()).thenReturn(true);
- when(owner.isInitialized()).thenReturn(true);
when(owner.shouldNotifyBridgeClients()).thenReturn(true);
+ when(owner.lockWhenRegionIsInitializing()).thenCallRealMethod();
initialize(owner, new Attributes(), null, false);
}
+
+ protected TxTestableAbstractRegionMap() {
+ this(true);
+ }
}
@Test
@@ -1361,6 +1380,120 @@ public class AbstractRegionMapTest {
false);
}
+ @Test
+ public void txApplyPutDoesNotLockWhenRegionIsInitialized() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap();
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+ EventID eventId = mock(EventID.class);
+ TXRmtEvent txRmtEvent = mock(TXRmtEvent.class);
+
+ arm.txApplyPut(Operation.UPDATE, KEY, "", false, txId, txRmtEvent,
eventId, null,
+ new ArrayList<>(), null, null, null, null, 1);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isFalse();
+ verify(arm._getOwner(), never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void txApplyPutLockWhenRegionIsInitializing() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap(false);
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+ EventID eventId = mock(EventID.class);
+ TXRmtEvent txRmtEvent = mock(TXRmtEvent.class);
+
+ arm.txApplyPut(Operation.UPDATE, KEY, "", false, txId, txRmtEvent,
eventId, null,
+ new ArrayList<>(), null, null, null, null, 1);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isTrue();
+ verify(arm._getOwner()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void txApplyDestroyDoesNotLockWhenRegionIsInitialized() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap();
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+
+ arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new
ArrayList<>(), null,
+ null, true, null, null, 0);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isFalse();
+ verify(arm._getOwner(), never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void txApplyDestroyLockWhenRegionIsInitializing() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap(false);
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+
+ arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new
ArrayList<>(), null,
+ null, true, null, null, 0);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isTrue();
+ verify(arm._getOwner()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void txApplyInvalidateDoesNotLockWhenRegionIsInitialized() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap();
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+
+ arm.txApplyInvalidate(new Object(), Token.INVALID, false,
+ txId, mock(TXRmtEvent.class), false,
+ mock(EventID.class), null, new ArrayList<EntryEventImpl>(), null,
null, null, null, 1);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isFalse();
+ verify(arm._getOwner(), never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void txApplyInvalidateLockWhenRegionIsInitializing() {
+ AbstractRegionMap arm = new TxTestableAbstractRegionMap(false);
+ TXId txId = mock(TXId.class, RETURNS_DEEP_STUBS);
+
+ arm.txApplyInvalidate(new Object(), Token.INVALID, false,
+ txId, mock(TXRmtEvent.class), false,
+ mock(EventID.class), null, new ArrayList<EntryEventImpl>(), null,
null, null, null, 1);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isTrue();
+ verify(arm._getOwner()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void invalidateDoesNotLockWhenRegionIsInitialized() {
+ TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, true,
+ mock(ConcurrentMapWithReusableEntries.class),
mock(RegionEntryFactory.class),
+ mock(RegionEntry.class));
+ EntryEventImpl event = createEventForInvalidate(arm._getOwner());
+ when(arm._getOwner().isInitialized()).thenReturn(true);
+ when(arm._getOwner().lockWhenRegionIsInitializing()).thenCallRealMethod();
+ arm.invalidate(event, false, false, false);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isFalse();
+ verify(arm._getOwner(), never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void invalidateLocksWhenRegionIsInitializing() {
+ TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, true,
+ mock(ConcurrentMapWithReusableEntries.class),
mock(RegionEntryFactory.class),
+ mock(RegionEntry.class));
+ EntryEventImpl event = createEventForInvalidate(arm._getOwner());
+ when(arm._getOwner().isInitialized()).thenReturn(false);
+ when(arm._getOwner().lockWhenRegionIsInitializing()).thenCallRealMethod();
+ arm.invalidate(event, false, false, false);
+
+ verify(arm._getOwner()).lockWhenRegionIsInitializing();
+ assertThat(arm._getOwner().lockWhenRegionIsInitializing()).isTrue();
+ verify(arm._getOwner()).unlockWhenRegionIsInitializing();
+ }
+
private static class TxNoRegionEntryTestableAbstractRegionMap
extends TxTestableAbstractRegionMap {
@Override
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 cf27546..a256012 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
@@ -15,7 +15,6 @@
package org.apache.geode.internal.cache;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Matchers.anyObject;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
@@ -27,8 +26,6 @@ import static org.mockito.Mockito.when;
import org.junit.Test;
-import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
-import org.apache.geode.internal.cache.versions.RegionVersionVector;
public class DistributedRegionTest {
@@ -49,21 +46,17 @@ public class DistributedRegionTest {
@Test
public void cleanUpAfterFailedGIIHoldsLockForClear() {
DistributedRegion distributedRegion = mock(DistributedRegion.class,
RETURNS_DEEP_STUBS);
- RegionVersionVector regionVersionVector = mock(RegionVersionVector.class);
RegionMap regionMap = mock(RegionMap.class);
- InternalDistributedMember member = mock(InternalDistributedMember.class);
doCallRealMethod().when(distributedRegion).cleanUpAfterFailedGII(false);
- when(distributedRegion.getVersionVector()).thenReturn(regionVersionVector);
when(distributedRegion.getRegionMap()).thenReturn(regionMap);
when(regionMap.isEmpty()).thenReturn(false);
- when(distributedRegion.getMyId()).thenReturn(member);
distributedRegion.cleanUpAfterFailedGII(false);
- verify(regionVersionVector).lockForClear(any(), any(), eq(member));
+ verify(distributedRegion).lockFailedGIIClearWriteLock();
verify(distributedRegion).closeEntries();
- verify(regionVersionVector).unlockForClear(eq(member));
+ verify(distributedRegion).unlockFailedGIIClearWriteLock();
}
@Test
@@ -77,8 +70,27 @@ public class DistributedRegionTest {
distributedRegion.cleanUpAfterFailedGII(true);
-
verify(diskRegion).resetRecoveredEntries(eq(distributedRegion));
verify(distributedRegion, never()).closeEntries();
}
+
+ @Test
+ public void lockHeldWhenRegionIsNotInitialized() {
+ DistributedRegion distributedRegion = mock(DistributedRegion.class);
+ doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
+ when(distributedRegion.isInitialized()).thenReturn(false);
+
+ assertThat(distributedRegion.lockWhenRegionIsInitializing()).isTrue();
+ verify(distributedRegion).lockFailedGIIClearReadLock();
+ }
+
+ @Test
+ public void lockNotHeldWhenRegionIsInitialized() {
+ DistributedRegion distributedRegion = mock(DistributedRegion.class);
+ doCallRealMethod().when(distributedRegion).lockWhenRegionIsInitializing();
+ when(distributedRegion.isInitialized()).thenReturn(true);
+
+ assertThat(distributedRegion.lockWhenRegionIsInitializing()).isFalse();
+ verify(distributedRegion, never()).lockFailedGIIClearReadLock();
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
index f02468d..4fdab13 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
@@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
@@ -42,6 +43,7 @@ import org.apache.geode.cache.EntryNotFoundException;
import org.apache.geode.cache.Operation;
import org.apache.geode.cache.query.internal.index.IndexManager;
import org.apache.geode.internal.cache.CachePerfStats;
+import org.apache.geode.internal.cache.DistributedRegion;
import org.apache.geode.internal.cache.EntryEventImpl;
import org.apache.geode.internal.cache.InternalRegion;
import org.apache.geode.internal.cache.RegionClearedException;
@@ -1404,6 +1406,40 @@ public class RegionMapDestroyTest {
verifyInvokedDestroyMethodsOnRegion(false);
}
+ @Test
+ public void destroyDoesNotLockGIIClearLockWhenRegionIsInitialized()
+ throws Exception {
+ DistributedRegion region = mock(DistributedRegion.class,
RETURNS_DEEP_STUBS);
+ when(region.isInitialized()).thenReturn(true);
+ when(region.lockWhenRegionIsInitializing()).thenCallRealMethod();
+ RegionMapDestroy mapDestroy =
+ new RegionMapDestroy(region, regionMap,
mock(CacheModificationLock.class));
+
+ mapDestroy.destroy(event, inTokenMode, duringRI, cacheWrite, isEviction,
+ expectedOldValue, removeRecoveredEntry);
+
+ verify(region).lockWhenRegionIsInitializing();
+ assertThat(region.lockWhenRegionIsInitializing()).isFalse();
+ verify(region, never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void destroyLockGIIClearLockWhenRegionIsInitializing()
+ throws Exception {
+ DistributedRegion region = mock(DistributedRegion.class,
RETURNS_DEEP_STUBS);
+ when(region.isInitialized()).thenReturn(false);
+ when(region.lockWhenRegionIsInitializing()).thenCallRealMethod();
+ RegionMapDestroy mapDestroy =
+ new RegionMapDestroy(region, regionMap,
mock(CacheModificationLock.class));
+
+ mapDestroy.destroy(event, inTokenMode, duringRI, cacheWrite, isEviction,
+ expectedOldValue, removeRecoveredEntry);
+
+ verify(region).lockWhenRegionIsInitializing();
+ assertThat(region.lockWhenRegionIsInitializing()).isTrue();
+ verify(region).unlockWhenRegionIsInitializing();
+ }
+
///////////////////// given methods /////////////////////////////
private void givenConcurrencyChecks(boolean enabled) {
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
index e525785..7f518dd 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
@@ -44,6 +44,7 @@ import org.apache.geode.cache.Operation;
import org.apache.geode.cache.RegionAttributes;
import org.apache.geode.cache.Scope;
import org.apache.geode.internal.cache.CachePerfStats;
+import org.apache.geode.internal.cache.DistributedRegion;
import org.apache.geode.internal.cache.EntryEventImpl;
import org.apache.geode.internal.cache.EntryEventSerialization;
import org.apache.geode.internal.cache.ImageState;
@@ -944,6 +945,41 @@ public class RegionMapPutTest {
eq(true), eq(ifNew), eq(ifOld), eq(expectedOldValue),
eq(requireOldValue));
}
+ @Test
+ public void
runWileLockedForCacheModificationDoesNotLockGIIClearLockWhenRegionIsInitialized()
+ throws Exception {
+ DistributedRegion region = mock(DistributedRegion.class);
+ when(region.isInitialized()).thenReturn(true);
+ when(region.lockWhenRegionIsInitializing()).thenCallRealMethod();
+ RegionMapPut regionMapPut = new RegionMapPut(focusedRegionMap, region,
cacheModificationLock,
+ entryEventSerialization, event, ifNew, ifOld, overwriteDestroyed,
requireOldValue,
+ expectedOldValue);
+
+ regionMapPut.runWhileLockedForCacheModification(() -> {
+ });
+
+ verify(region).lockWhenRegionIsInitializing();
+ assertThat(region.lockWhenRegionIsInitializing()).isFalse();
+ verify(region, never()).unlockWhenRegionIsInitializing();
+ }
+
+ @Test
+ public void
runWileLockedForCacheModificationLockGIIClearLockWhenRegionIsInitializing() {
+ DistributedRegion region = mock(DistributedRegion.class);
+ when(region.isInitialized()).thenReturn(false);
+ when(region.lockWhenRegionIsInitializing()).thenCallRealMethod();
+ RegionMapPut regionMapPut = new RegionMapPut(focusedRegionMap, region,
cacheModificationLock,
+ entryEventSerialization, event, ifNew, ifOld, overwriteDestroyed,
requireOldValue,
+ expectedOldValue);
+
+ regionMapPut.runWhileLockedForCacheModification(() -> {
+ });
+
+ verify(region).lockWhenRegionIsInitializing();
+ assertThat(region.lockWhenRegionIsInitializing()).isTrue();
+ verify(region).unlockWhenRegionIsInitializing();
+ }
+
private void givenAnOperationThatDoesNotGuaranteeOldValue() {
when(event.getOperation()).thenReturn(Operation.UPDATE);
}
