This is an automated email from the ASF dual-hosted git repository.
sai_boorlagadda pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 373f7ad GEODE-5291: More destroys recorded in CachePerfStats than
actual in a transaction with expiration (#2028)
373f7ad is described below
commit 373f7ad96e487f881d25293c49dde572f61273e1
Author: Sai Boorlagadda <[email protected]>
AuthorDate: Wed Jun 6 12:49:20 2018 -0700
GEODE-5291: More destroys recorded in CachePerfStats than actual in a
transaction with expiration (#2028)
---
.../geode/internal/cache/AbstractRegionMap.java | 9 +-
.../apache/geode/internal/cache/LocalRegion.java | 5 +-
.../geode/internal/cache/ProxyRegionMap.java | 2 +-
.../cache/AbstractRegionMapTxApplyDestroyTest.java | 166 +++++++++++++++++++--
4 files changed, 159 insertions(+), 23 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 1b03d84..3155d79 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
@@ -1126,7 +1126,8 @@ public abstract class AbstractRegionMap
clearOccured = true;
}
owner.txApplyDestroyPart2(re, re.getKey(), inTokenMode,
- clearOccured /* Clear Conflciting with the operation */);
+ clearOccured /* Clear Conflciting with the operation */,
+ wasDestroyedOrRemoved);
boolean invokeCallbacks = shouldInvokeCallbacks(owner,
isRegionReady || inRI);
if (invokeCallbacks) {
switchEventOwnerAndOriginRemote(callbackEvent,
hasRemoteOrigin);
@@ -1211,7 +1212,7 @@ public abstract class AbstractRegionMap
owner.updateSizeOnRemove(oldRe.getKey(), oldSize);
}
owner.txApplyDestroyPart2(oldRe, oldRe.getKey(),
inTokenMode,
- false /* Clear Conflicting with the operation */);
+ false /* Clear Conflicting with the operation */,
wasDestroyedOrRemoved);
lruEntryDestroy(oldRe);
} finally {
if (!callbackEventAddedToPending)
@@ -1219,7 +1220,7 @@ public abstract class AbstractRegionMap
}
} catch (RegionClearedException rce) {
owner.txApplyDestroyPart2(oldRe, oldRe.getKey(),
inTokenMode,
- true /* Clear Conflicting with the operation */);
+ true /* Clear Conflicting with the operation */, true);
}
if (owner.getConcurrencyChecksEnabled()
&& callbackEvent.getVersionTag() != null) {
@@ -1266,7 +1267,7 @@ public abstract class AbstractRegionMap
removeEntry(key, newRe, false);
}
owner.txApplyDestroyPart2(newRe, newRe.getKey(), inTokenMode,
- false /* clearConflict */);
+ false /* clearConflict */, true);
// Note no need for LRU work since the entry is destroyed
// and will be removed when gii completes
} finally {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index ec65991..1642a8a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -6887,13 +6887,14 @@ public class LocalRegion extends AbstractRegion
implements LoaderHelperFactory,
* Called by lower levels, while still holding the write sync lock, and the
low level has
* completed its part of the basic destroy
*/
- void txApplyDestroyPart2(RegionEntry re, Object key, boolean inTokenMode,
boolean clearConflict) {
+ void txApplyDestroyPart2(RegionEntry re, Object key, boolean inTokenMode,
boolean clearConflict,
+ boolean alreadyDestroyedOrRemoved) {
if (this.testCallable != null) {
this.testCallable.call(this, Operation.DESTROY, re);
}
if (inTokenMode) {
getImageState().addDestroyedEntry(key);
- } else {
+ } else if (!alreadyDestroyedOrRemoved) {
updateStatsForDestroy();
}
if (this.entryUserAttributes != null) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java
index 6eb4479..a03e817 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java
@@ -266,7 +266,7 @@ class ProxyRegionMap implements RegionMap {
ClientProxyMembershipID bridgeContext, boolean isOperationRemote,
TXEntryState txEntryState,
VersionTag versionTag, long tailKey) {
this.owner.txApplyDestroyPart2(markerEntry, key, inTokenMode,
- false /* Clear conflict occurred */);
+ false /* Clear conflict occurred */, false);
if (!inTokenMode) {
if (event != null) {
event.addDestroy(this.owner, markerEntry, key, aCallbackArgument);
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
index 6c8544a..1d649cd 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
@@ -251,7 +251,7 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).isEmpty();
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key),
- eq(this.inTokenMode), eq(false));
+ eq(this.inTokenMode), eq(false), eq(false));
}
@Test
@@ -374,7 +374,7 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
- verify(owner, times(1)).txApplyDestroyPart2(any(), any(), anyBoolean(),
eq(true));
+ verify(owner, times(1)).txApplyDestroyPart2(any(), any(), anyBoolean(),
eq(true), anyBoolean());
verify(regionMap, never()).lruEntryDestroy(any());
}
@@ -387,7 +387,8 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
- verify(owner, times(1)).txApplyDestroyPart2(any(), any(), anyBoolean(),
eq(false));
+ verify(owner, times(1)).txApplyDestroyPart2(any(), any(), anyBoolean(),
eq(false),
+ anyBoolean());
verify(regionMap, times(1)).lruEntryDestroy(any());
}
@@ -484,7 +485,23 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).hasSize(1);
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+
+ }
+
+ @Test
+ public void
txApplyDestroyHasPendingCallback_givenExistingRemovedRegionEntryWithoutInTokenModeAndNotInRI()
{
+ givenLocalRegion();
+ givenConcurrencyChecks();
+ givenExistingRemovedRegionEntry();
+ inTokenMode = false;
+ inRI = false;
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).hasSize(1);
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@@ -500,7 +517,22 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).hasSize(1);
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasPendingCallback_givenExistingRemovedRegionEntryWithoutInTokenModeAndWithInRI()
{
+ givenLocalRegion();
+ givenConcurrencyChecks();
+ givenExistingRemovedRegionEntry();
+ inTokenMode = false;
+ inRI = true;
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).hasSize(1);
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -515,7 +547,22 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).hasSize(1);
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasPendingCallback_givenExistingRemovedRegionEntryWithInTokenModeAndInRI()
{
+ givenLocalRegion();
+ givenConcurrencyChecks();
+ givenExistingRemovedRegionEntry();
+ inTokenMode = true;
+ inRI = true;
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).hasSize(1);
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -527,7 +574,19 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).isEmpty();
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasNoPendingCallback_givenExistingRemovedRegionEntryWithPartitionedRegion()
{
+ givenBucketRegion();
+ givenExistingRemovedRegionEntry();
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).isEmpty();
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -589,7 +648,20 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).isEmpty();
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasNoPendingCallback_givenExistingRemovedRegionEntryWithoutConcurrencyChecks()
{
+ givenLocalRegion();
+ givenExistingRemovedRegionEntry();
+ givenNoConcurrencyChecks();
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).isEmpty();
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -602,7 +674,20 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).hasSize(1);
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasPendingCallback_givenExistingRemovedRegionEntryWithShouldDispatchListenerEvent()
{
+ givenLocalRegion();
+ givenExistingRemovedRegionEntry();
+ when(owner.shouldDispatchListenerEvent()).thenReturn(true);
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).hasSize(1);
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -615,11 +700,24 @@ public class AbstractRegionMapTxApplyDestroyTest {
assertThat(pendingCallbacks).hasSize(1);
verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyHasPendingCallback_givenExistingRemovedRegionEntryWithshouldNotifyBridgeClients()
{
+ givenLocalRegion();
+ givenExistingRemovedRegionEntry();
+ when(owner.shouldNotifyBridgeClients()).thenReturn(true);
+
+ doTxApplyDestroy();
+
+ assertThat(pendingCallbacks).hasSize(1);
+ verify(owner, times(1)).txApplyDestroyPart2(same(existingRegionEntry),
eq(key), eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
- public void
txApplyDestroyDoesNotCallsTxApplyDestroyPart2_givenExistingRegionEntry() {
+ public void
txApplyDestroyDoesCallTxApplyDestroyPart2_givenFactoryRegionEntry() {
givenLocalRegion();
givenConcurrencyChecks();
givenFactoryRegionEntry();
@@ -628,7 +726,7 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
verify(owner, times(1)).txApplyDestroyPart2(same(factoryRegionEntry),
eq(key), eq(inTokenMode),
- eq(false));
+ eq(false), eq(true));
}
// tests for "factoryRegionEntry" (that is, no existing region entry and no
oldRegionEntry).
@@ -973,7 +1071,8 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
- verify(owner, never()).txApplyDestroyPart2(any(), any(), anyBoolean(),
anyBoolean());
+ verify(owner, never()).txApplyDestroyPart2(any(), any(), anyBoolean(),
anyBoolean(),
+ anyBoolean());
}
// tests for "oldRegionEntry" (that is, an existing region entry found by
putIfAbsent).
@@ -1296,7 +1395,19 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
verify(owner, times(1)).txApplyDestroyPart2(same(oldRegionEntry), eq(key),
eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyCallsTxApplyDestroyPart2_givenOldRemovedRegionEntry() {
+ givenLocalRegion();
+ givenConcurrencyChecks();
+ givenOldRemovedRegionEntry();
+
+ doTxApplyDestroy();
+
+ verify(owner, times(1)).txApplyDestroyPart2(same(oldRegionEntry), eq(key),
eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -1309,7 +1420,20 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
verify(owner, times(1)).txApplyDestroyPart2(same(oldRegionEntry), eq(key),
eq(inTokenMode),
- eq(false));
+ eq(false), eq(false));
+ }
+
+ @Test
+ public void
txApplyDestroyCallsTxApplyDestroyPart2_givenOldRemovedRegionEntryWithInTokenMode()
{
+ givenLocalRegion();
+ givenConcurrencyChecks();
+ givenOldRemovedRegionEntry();
+ inTokenMode = true;
+
+ doTxApplyDestroy();
+
+ verify(owner, times(1)).txApplyDestroyPart2(same(oldRegionEntry), eq(key),
eq(inTokenMode),
+ eq(false), eq(true));
}
@Test
@@ -1346,7 +1470,7 @@ public class AbstractRegionMapTxApplyDestroyTest {
doTxApplyDestroy();
verify(owner, times(1)).txApplyDestroyPart2(same(oldRegionEntry), eq(key),
eq(inTokenMode),
- eq(true));
+ eq(true), eq(true));
}
@Test
@@ -1415,6 +1539,11 @@ public class AbstractRegionMapTxApplyDestroyTest {
when(entryMap.get(eq(key))).thenReturn(existingRegionEntry);
}
+ private void givenExistingRemovedRegionEntry() {
+ givenExistingRegionEntry();
+ when(existingRegionEntry.isDestroyedOrRemoved()).thenReturn(true);
+ }
+
private void givenFactoryRegionEntry() {
when(entryMap.get(eq(key))).thenReturn(null);
when(regionEntryFactory.createEntry(any(), any(),
any())).thenReturn(factoryRegionEntry);
@@ -1426,6 +1555,11 @@ public class AbstractRegionMapTxApplyDestroyTest {
when(entryMap.putIfAbsent(eq(key),
same(factoryRegionEntry))).thenReturn(oldRegionEntry);
}
+ private void givenOldRemovedRegionEntry() {
+ givenOldRegionEntry();
+ when(oldRegionEntry.isDestroyedOrRemoved()).thenReturn(true);
+ }
+
private void givenNotInTokenMode() {
inTokenMode = false;
}
--
To stop receiving notification emails like this one, please contact
[email protected].
