Skip to content

Commit

Permalink
Revert "GEODE-5265: fix dataStoreEntryCount statistic (#2006)"
Browse files Browse the repository at this point in the history 
This reverts commit 44e5d36.
  • Loading branch information
@dschneider-pivotal
dschneider-pivotal committed Jun 1, 2018
1 parent 7f63c4b commit 7d69f29
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 364 deletions.
23 changes: 6 additions & 17 deletions geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,6 @@ public RegionEntry updateRecoveredEntry(Object key, DiskEntry.RecoveredEntry val
try {
if (_isOwnerALocalRegion()) {
boolean oldValueWasTombstone = re.isTombstone();
boolean oldIsDestroyedOrRemoved = re.isDestroyedOrRemoved();
if (oldValueWasTombstone) {
// when a tombstone is to be overwritten, unschedule it first
_getOwner().unscheduleTombstone(re);
Expand All @@ -784,10 +783,8 @@ public RegionEntry updateRecoveredEntry(Object key, DiskEntry.RecoveredEntry val
// value for the cache
if (re.isTombstone()) {
_getOwner().scheduleTombstone(re, re.getVersionStamp().asVersionTag());
if (!oldIsDestroyedOrRemoved) {
_getOwner().updateSizeOnRemove(key, oldSize);
}
} else if (oldIsDestroyedOrRemoved) {
_getOwner().updateSizeOnRemove(key, oldSize);
} else if (oldValueWasTombstone) {
_getOwner().updateSizeOnCreate(key, _getOwner().calculateRegionEntryValueSize(re));
} else {
_getOwner().updateSizeOnPut(key, oldSize,
Expand Down Expand Up @@ -873,7 +870,6 @@ public boolean initialImagePut(final Object key, final long lastModified, Object
}
}
final boolean oldIsTombstone = oldRe.isTombstone();
final boolean oldIsDestroyedOrRemoved = oldRe.isDestroyedOrRemoved();
final int oldSize = owner.calculateRegionEntryValueSize(oldRe);
try {
result = oldRe.initialImagePut(owner, lastModified, newValue, wasRecovered,
Expand All @@ -888,9 +884,7 @@ public boolean initialImagePut(final Object key, final long lastModified, Object
}
}
if (newValue == Token.TOMBSTONE) {
if (!oldIsDestroyedOrRemoved) {
owner.updateSizeOnRemove(key, oldSize);
}
owner.updateSizeOnRemove(key, oldSize);
if (owner.getServerProxy() == null
&& owner.getVersionVector().isTombstoneTooOld(
entryVersion.getMemberID(), entryVersion.getRegionVersion())) {
Expand Down Expand Up @@ -1071,7 +1065,6 @@ public void txApplyDestroy(Object key, TransactionId txId, TXRmtEvent txEvent,
if (!re.isRemoved() || re.isTombstone()) {
Object oldValue = re.getValueInVM(owner);
final int oldSize = owner.calculateRegionEntryValueSize(re);
final boolean wasDestroyedOrRemoved = re.isDestroyedOrRemoved();
// Create an entry event only if the calling context is
// a receipt of a TXCommitMessage AND there are callbacks installed
// for this region
Expand Down Expand Up @@ -1119,9 +1112,7 @@ public void txApplyDestroy(Object key, TransactionId txId, TXRmtEvent txEvent,
}
}
EntryLogger.logTXDestroy(_getOwnerObject(), key);
if (!wasDestroyedOrRemoved) {
owner.updateSizeOnRemove(key, oldSize);
}
owner.updateSizeOnRemove(key, oldSize);
} catch (RegionClearedException rce) {
clearOccured = true;
}
Expand Down Expand Up @@ -1194,7 +1185,6 @@ public void txApplyDestroy(Object key, TransactionId txId, TXRmtEvent txEvent,
}
int oldSize = 0;
boolean wasTombstone = oldRe.isTombstone();
boolean wasDestroyedOrRemoved = oldRe.isDestroyedOrRemoved();
{
if (!wasTombstone) {
oldSize = owner.calculateRegionEntryValueSize(oldRe);
Expand All @@ -1207,9 +1197,7 @@ public void txApplyDestroy(Object key, TransactionId txId, TXRmtEvent txEvent,
if (wasTombstone) {
owner.unscheduleTombstone(oldRe);
}
if (!wasDestroyedOrRemoved) {
owner.updateSizeOnRemove(oldRe.getKey(), oldSize);
}
owner.updateSizeOnRemove(oldRe.getKey(), oldSize);
owner.txApplyDestroyPart2(oldRe, oldRe.getKey(), inTokenMode,
false /* Clear Conflicting with the operation */);
lruEntryDestroy(oldRe);
Expand Down Expand Up @@ -1257,6 +1245,7 @@ public void txApplyDestroy(Object key, TransactionId txId, TXRmtEvent txEvent,
callbackEventAddedToPending = true;
}
EntryLogger.logTXDestroy(_getOwnerObject(), key);
owner.updateSizeOnCreate(newRe.getKey(), 0);
if (owner.getConcurrencyChecksEnabled() && callbackEvent.getVersionTag() != null) {
newRe.makeTombstone(owner, callbackEvent.getVersionTag());
} else if (!inTokenMode) {
Expand Down
5 changes: 1 addition & 4 deletions geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,14 +720,11 @@ private boolean destroyEntry(RegionEntry re, EntryEventImpl event, boolean inTok
EntryNotFoundException, RegionClearedException {
focusedRegionMap.processVersionTag(re, event);
final int oldSize = internalRegion.calculateRegionEntryValueSize(re);
final boolean wasRemoved = re.isDestroyedOrRemoved();
boolean retVal = re.destroy(event.getRegion(), event, inTokenMode, cacheWrite, expectedOldValue,
forceDestroy, removeRecoveredEntry);
if (retVal) {
EntryLogger.logDestroy(event);
if (!wasRemoved) {
internalRegion.updateSizeOnRemove(event.getKey(), oldSize);
}
internalRegion.updateSizeOnRemove(event.getKey(), oldSize);
}
return retVal;
}
Expand Down
212 changes: 0 additions & 212 deletions geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
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.doThrow;
Expand All @@ -45,17 +44,13 @@
import org.apache.geode.cache.Operation;
import org.apache.geode.cache.Scope;
import org.apache.geode.cache.TransactionId;
import org.apache.geode.cache.client.internal.ServerRegionProxy;
import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.internal.cache.entries.DiskEntry.RecoveredEntry;
import org.apache.geode.internal.cache.eviction.EvictableEntry;
import org.apache.geode.internal.cache.eviction.EvictionController;
import org.apache.geode.internal.cache.eviction.EvictionCounters;
import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
import org.apache.geode.internal.cache.versions.RegionVersionVector;
import org.apache.geode.internal.cache.versions.VersionHolder;
import org.apache.geode.internal.cache.versions.VersionSource;
import org.apache.geode.internal.cache.versions.VersionStamp;
import org.apache.geode.internal.cache.versions.VersionTag;
import org.apache.geode.internal.util.concurrent.ConcurrentMapWithReusableEntries;
import org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap;
Expand Down Expand Up @@ -716,196 +711,6 @@ public void txApplyInvalidateDoesNotInvalidateRemovedToken() throws RegionCleare
}
}

@Test
public void updateRecoveredEntry_givenExistingDestroyedOrRemovedAndSettingToTombstone_neverCallsUpdateSizeOnRemove() {
RecoveredEntry recoveredEntry = mock(RecoveredEntry.class);
RegionEntry regionEntry = mock(RegionEntry.class);
when(regionEntry.isTombstone()).thenReturn(false).thenReturn(true);
when(regionEntry.isDestroyedOrRemoved()).thenReturn(true);
when(regionEntry.getVersionStamp()).thenReturn(mock(VersionStamp.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, regionEntry);

arm.updateRecoveredEntry(KEY, recoveredEntry);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void updateRecoveredEntry_givenExistingRemovedNonTombstone_neverCallsUpdateSizeOnRemove() {
RecoveredEntry recoveredEntry = mock(RecoveredEntry.class);
RegionEntry regionEntry = mock(RegionEntry.class);
when(regionEntry.isRemoved()).thenReturn(true);
when(regionEntry.isTombstone()).thenReturn(false);
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, regionEntry);

arm.updateRecoveredEntry(KEY, recoveredEntry);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void updateRecoveredEntry_givenNoExistingEntry_neverCallsUpdateSizeOnRemove() {
RecoveredEntry recoveredEntry = mock(RecoveredEntry.class);
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, null);

arm.updateRecoveredEntry(KEY, recoveredEntry);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void updateRecoveredEntry_givenExistingNonTombstoneAndSettingToTombstone_callsUpdateSizeOnRemove() {
RecoveredEntry recoveredEntry = mock(RecoveredEntry.class);
RegionEntry regionEntry = mock(RegionEntry.class);
when(regionEntry.isTombstone()).thenReturn(false).thenReturn(true);
when(regionEntry.getVersionStamp()).thenReturn(mock(VersionStamp.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, regionEntry);

arm.updateRecoveredEntry(KEY, recoveredEntry);

verify(arm._getOwner(), times(1)).updateSizeOnRemove(eq(KEY), anyInt());
}

@Test
public void initialImagePut_givenPutIfAbsentReturningDestroyedOrRemovedEntry_neverCallsUpdateSizeOnRemove()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
when(entry.isDestroyedOrRemoved()).thenReturn(true);
when(entry.initialImagePut(any(), anyLong(), any(), anyBoolean(), anyBoolean()))
.thenReturn(true);
VersionStamp versionStamp = mock(VersionStamp.class);
when(entry.getVersionStamp()).thenReturn(versionStamp);
when(versionStamp.asVersionTag()).thenReturn(mock(VersionTag.class));
when(map.putIfAbsent(eq(KEY), any())).thenReturn(entry).thenReturn(null);
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, map, null);
when(arm._getOwner().getConcurrencyChecksEnabled()).thenReturn(true);
when(arm._getOwner().getServerProxy()).thenReturn(mock(ServerRegionProxy.class));
VersionTag versionTag = mock(VersionTag.class);
when(versionTag.getMemberID()).thenReturn(mock(VersionSource.class));

arm.initialImagePut(KEY, 0, Token.TOMBSTONE, false, false, versionTag, null, false);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void initialImagePut_givenPutIfAbsentReturningNonTombstone_callsUpdateSizeOnRemove()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
when(entry.isTombstone()).thenReturn(false);
when(entry.isDestroyedOrRemoved()).thenReturn(false);
when(entry.initialImagePut(any(), anyLong(), any(), anyBoolean(), anyBoolean()))
.thenReturn(true);
VersionStamp versionStamp = mock(VersionStamp.class);
when(entry.getVersionStamp()).thenReturn(versionStamp);
when(versionStamp.asVersionTag()).thenReturn(mock(VersionTag.class));
when(map.putIfAbsent(eq(KEY), any())).thenReturn(entry).thenReturn(null);
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, map, null);
when(arm._getOwner().getConcurrencyChecksEnabled()).thenReturn(true);
when(arm._getOwner().getServerProxy()).thenReturn(mock(ServerRegionProxy.class));
VersionTag versionTag = mock(VersionTag.class);
when(versionTag.getMemberID()).thenReturn(mock(VersionSource.class));

arm.initialImagePut(KEY, 0, Token.TOMBSTONE, false, false, versionTag, null, false);

verify(arm._getOwner(), times(1)).updateSizeOnRemove(eq(KEY), anyInt());
}

@Test
public void txApplyDestroy_givenExistingDestroyedOrRemovedEntry_neverCallsUpdateSizeOnRemove() {
RegionEntry regionEntry = mock(RegionEntry.class);
when(regionEntry.isTombstone()).thenReturn(false);
when(regionEntry.isDestroyedOrRemoved()).thenReturn(true);
when(regionEntry.getVersionStamp()).thenReturn(mock(VersionStamp.class));
TXId txId = mock(TXId.class);
when(txId.getMemberId()).thenReturn(mock(InternalDistributedMember.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, regionEntry);

arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new ArrayList<>(), null,
null, false, null, null, 0);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void txApplyDestroy_givenExistingNonTombstone_callsUpdateSizeOnRemove() {
RegionEntry regionEntry = mock(RegionEntry.class);
when(regionEntry.isTombstone()).thenReturn(false);
when(regionEntry.getVersionStamp()).thenReturn(mock(VersionStamp.class));
TXId txId = mock(TXId.class);
when(txId.getMemberId()).thenReturn(mock(InternalDistributedMember.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, null, null, regionEntry);

arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new ArrayList<>(), null,
null, false, null, null, 0);

verify(arm._getOwner(), times(1)).updateSizeOnRemove(eq(KEY), anyInt());
}

@Test
public void txApplyDestroy_givenPutIfAbsentReturningDestroyedOrRemovedEntry_neverCallsUpdateSizeOnRemove()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
when(entry.isTombstone()).thenReturn(false);
when(entry.isDestroyedOrRemoved()).thenReturn(true);
VersionStamp versionStamp = mock(VersionStamp.class);
when(entry.getVersionStamp()).thenReturn(versionStamp);
when(versionStamp.asVersionTag()).thenReturn(mock(VersionTag.class));
when(map.putIfAbsent(eq(KEY), any())).thenReturn(entry).thenReturn(null);
TXId txId = mock(TXId.class);
when(txId.getMemberId()).thenReturn(mock(InternalDistributedMember.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, map, null);
when(arm._getOwner().getConcurrencyChecksEnabled()).thenReturn(true);

arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new ArrayList<>(), null,
null, false, null, null, 0);

verify(arm._getOwner(), never()).updateSizeOnRemove(any(), anyInt());
}

@Test
public void txApplyDestroy_givenPutIfAbsentReturningNonTombstone_callsUpdateSizeOnRemove()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
when(entry.getKey()).thenReturn(KEY);
when(entry.isTombstone()).thenReturn(false);
VersionStamp versionStamp = mock(VersionStamp.class);
when(entry.getVersionStamp()).thenReturn(versionStamp);
when(versionStamp.asVersionTag()).thenReturn(mock(VersionTag.class));
when(map.putIfAbsent(eq(KEY), any())).thenReturn(entry).thenReturn(null);
TXId txId = mock(TXId.class);
when(txId.getMemberId()).thenReturn(mock(InternalDistributedMember.class));
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, map, null);
when(arm._getOwner().getConcurrencyChecksEnabled()).thenReturn(true);

arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new ArrayList<>(), null,
null, false, null, null, 0);

verify(arm._getOwner(), times(1)).updateSizeOnRemove(eq(KEY), anyInt());
}

@Test
public void txApplyDestroy_givenFactory_neverCallsUpdateSizeOnRemove()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
TXId txId = mock(TXId.class);
when(txId.getMemberId()).thenReturn(mock(InternalDistributedMember.class));
RegionEntryFactory factory = mock(RegionEntryFactory.class);
when(factory.createEntry(any(), any(), any())).thenReturn(entry);
TestableAbstractRegionMap arm = new TestableAbstractRegionMap(false, map, factory);
when(arm._getOwner().getConcurrencyChecksEnabled()).thenReturn(true);

arm.txApplyDestroy(KEY, txId, null, false, false, null, null, null, new ArrayList<>(), null,
null, false, null, null, 0);

verify(arm._getOwner(), never()).updateSizeOnCreate(any(), anyInt());
}

private EntryEventImpl createEventForInvalidate(LocalRegion lr) {
when(lr.getKeyInfo(KEY)).thenReturn(new KeyInfo(KEY, null, null));
return EntryEventImpl.create(lr, Operation.INVALIDATE, KEY, false, null, true, false);
Expand Down Expand Up @@ -938,7 +743,6 @@ private void verifyTxApplyInvalidate(TxTestableAbstractRegionMap arm, Object key
* TestableAbstractRegionMap
*/
private static class TestableAbstractRegionMap extends AbstractRegionMap {
private final RegionEntry regionEntryForGetEntry;

protected TestableAbstractRegionMap() {
this(false);
Expand All @@ -950,14 +754,7 @@ protected TestableAbstractRegionMap(boolean withConcurrencyChecks) {

protected TestableAbstractRegionMap(boolean withConcurrencyChecks,
ConcurrentMapWithReusableEntries map, RegionEntryFactory factory) {
this(withConcurrencyChecks, map, factory, null);
}

protected TestableAbstractRegionMap(boolean withConcurrencyChecks,
ConcurrentMapWithReusableEntries map, RegionEntryFactory factory,
RegionEntry regionEntryForGetEntry) {
super(null);
this.regionEntryForGetEntry = regionEntryForGetEntry;
LocalRegion owner = mock(LocalRegion.class);
CachePerfStats cachePerfStats = mock(CachePerfStats.class);
when(owner.getCachePerfStats()).thenReturn(cachePerfStats);
Expand All @@ -974,15 +771,6 @@ protected TestableAbstractRegionMap(boolean withConcurrencyChecks,
setEntryFactory(factory);
}
}

@Override
public RegionEntry getEntry(Object key) {
if (this.regionEntryForGetEntry != null) {
return this.regionEntryForGetEntry;
} else {
return super.getEntry(key);
}
}
}


Expand Down
Loading

0 comments on commit 7d69f29

Please sign in to comment.