refactored isTombstoneNotNeeded
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/cb56ade2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/cb56ade2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/cb56ade2 Branch: refs/heads/feature/GEODE-1420 Commit: cb56ade2de5efd6b6b0a10f52dac39b64d8a5e38 Parents: c5329e7 Author: Darrel Schneider <dschnei...@pivotal.io> Authored: Tue Jun 21 16:31:18 2016 -0700 Committer: Darrel Schneider <dschnei...@pivotal.io> Committed: Tue Jun 21 16:31:18 2016 -0700 ---------------------------------------------------------------------- .../internal/cache/AbstractRegionMap.java | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/cb56ade2/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java index bc919fc..0c906d9 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java @@ -3637,21 +3637,31 @@ public abstract class AbstractRegionMap implements RegionMap { public boolean isTombstoneNotNeeded(RegionEntry re, int destroyedVersion) { // no need for synchronization - stale values are okay here - RegionEntry actualRe = getEntry(re.getKey()); // TODO this looks like a problem for regionEntry pooling - if (actualRe != re) { // null actualRe is okay here - return true; // tombstone was evicted at some point + if ( getEntry(re.getKey()) != re) { + // region entry was either removed (null) + // or changed to a different region entry. + // In either case the old tombstone is no longer needed. + return true; + } + if (!re.isTombstone()) { + // if the region entry no longer contains a tombstone + // then the old tombstone is no longer needed + return true; } - VersionStamp vs = re.getVersionStamp(); + VersionStamp<?> vs = re.getVersionStamp(); if (vs == null) { // if we have no VersionStamp why were we even added as a tombstone? // We used to see an NPE here. See bug 52092. logger.error("Unexpected RegionEntry scheduled as tombstone: re.getClass {} destroyedVersion {}", re.getClass(), destroyedVersion); return true; } - int entryVersion = vs.getEntryVersion(); - boolean isSameTombstone = (entryVersion == destroyedVersion && re.isTombstone()); - return !isSameTombstone; + if (vs.getEntryVersion() != destroyedVersion) { + // the version changed so old tombstone no longer needed + return true; + } + // region entry still has the same tombstone so we need to keep it. + return false; } /** removes a tombstone that has expired locally */