Repository: incubator-geode Updated Branches: refs/heads/feature/GEODE-1420 895e799f0 -> 7c325a156
cleaned up the blockgclock code Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/2e4d4d2c Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/2e4d4d2c Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/2e4d4d2c Branch: refs/heads/feature/GEODE-1420 Commit: 2e4d4d2c158f798811b019f90c1b18756de51f4b Parents: 895e799 Author: Darrel Schneider <dschnei...@pivotal.io> Authored: Thu Jun 30 14:27:24 2016 -0700 Committer: Darrel Schneider <dschnei...@pivotal.io> Committed: Thu Jun 30 14:27:24 2016 -0700 ---------------------------------------------------------------------- .../internal/cache/InitialImageOperation.java | 2 +- .../internal/cache/TombstoneService.java | 77 +++++++++++++------- 2 files changed, 52 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2e4d4d2c/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java index 11cc030..7ee5c74 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java @@ -1637,7 +1637,7 @@ public class InitialImageOperation { } } if (this.checkTombstoneVersions && this.versionVector != null && rgn.concurrencyChecksEnabled) { - synchronized(rgn.getCache().getTombstoneService().blockGCLock) { + synchronized(rgn.getCache().getTombstoneService().getBlockGCLock()) { if (goWithFullGII(rgn, this.versionVector)) { if (isGiiDebugEnabled) { logger.trace(LogMarker.GII, "have to do fullGII"); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2e4d4d2c/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java index 238fe10..234454b 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java @@ -111,12 +111,9 @@ public class TombstoneService { * two sweepers, one for replicated regions (including PR buckets) and one for * other regions. They have different timeout intervals. */ - private final TombstoneSweeper replicatedTombstoneSweeper; - private final TombstoneSweeper nonReplicatedTombstoneSweeper; + private final ReplicateTombstoneSweeper replicatedTombstoneSweeper; + private final NonReplicateTombstoneSweeper nonReplicatedTombstoneSweeper; - public final Object blockGCLock = new Object(); - private int progressingDeltaGIICount; - public static TombstoneService initialize(GemFireCacheImpl cache) { TombstoneService instance = new TombstoneService(cache); // cache.getResourceManager().addResourceListener(instance); experimental @@ -176,21 +173,15 @@ public class TombstoneService { } public int getGCBlockCount() { - synchronized(this.blockGCLock) { - return this.progressingDeltaGIICount; - } + return replicatedTombstoneSweeper.getGCBlockCount(); } public int incrementGCBlockCount() { - synchronized(this.blockGCLock) { - return ++this.progressingDeltaGIICount; - } + return replicatedTombstoneSweeper.incrementGCBlockCount(); } public int decrementGCBlockCount() { - synchronized(this.blockGCLock) { - return --this.progressingDeltaGIICount; - } + return replicatedTombstoneSweeper.decrementGCBlockCount(); } /** @@ -199,7 +190,7 @@ public class TombstoneService { */ @SuppressWarnings("rawtypes") public Set<Object> gcTombstones(LocalRegion r, Map<VersionSource, Long> regionGCVersions, boolean needsKeys) { - synchronized(this.blockGCLock) { + synchronized(getBlockGCLock()) { int count = getGCBlockCount(); if (count > 0) { // if any delta GII is on going as provider at this member, not to do tombstone GC @@ -311,6 +302,9 @@ public class TombstoneService { return "Destroyed entries GC service. Replicate Queue=" + this.replicatedTombstoneSweeper + " Non-replicate Queue=" + this.nonReplicatedTombstoneSweeper; } + public Object getBlockGCLock() { + return this.replicatedTombstoneSweeper.getBlockGCLock(); + } private static class Tombstone extends CompactVersionHolder { // tombstone overhead size public static int PER_TOMBSTONE_OVERHEAD = ReflectionSingleObjectSizer.REFERENCE_SIZE // queue's reference to the tombstone @@ -403,6 +397,9 @@ public class TombstoneService { */ private volatile boolean batchExpirationInProgress; + private final Object blockGCLock = new Object(); + private int progressingDeltaGIICount; + /** * A test hook to force a call to expireBatch. * The call will only happen after testHook_forceExpirationCount @@ -421,16 +418,43 @@ public class TombstoneService { this.expiredTombstones = new ArrayList<Tombstone>(); } + public int decrementGCBlockCount() { + synchronized(getBlockGCLock()) { + return --progressingDeltaGIICount; + } + } + + public int incrementGCBlockCount() { + synchronized(getBlockGCLock()) { + return ++progressingDeltaGIICount; + } + } + + public int getGCBlockCount() { + synchronized(getBlockGCLock()) { + return progressingDeltaGIICount; + } + } + + public Object getBlockGCLock() { + return blockGCLock; + } + @Override protected boolean scanExpired(Predicate<Tombstone> predicate) { boolean result = false; long removalSize = 0; - for (int idx=expiredTombstones.size()-1; idx >= 0; idx--) { - Tombstone t = expiredTombstones.get(idx); - if (predicate.test(t)) { - removalSize += t.getSize(); - expiredTombstones.remove(idx); - result = true; + synchronized(getBlockGCLock()) { + // Iterate in reverse order to optimize lots of removes. + // Since expiredTombstones is an ArrayList removing from + // low indexes requires moving everything at a higher index down. + for (int idx=expiredTombstones.size()-1; idx >= 0; idx--) { + Tombstone t = expiredTombstones.get(idx); + if (predicate.test(t)) { + removalSize += t.getSize(); + expiredTombstones.remove(idx); + result = true; + } } } updateMemoryEstimate(-removalSize); @@ -444,8 +468,8 @@ public class TombstoneService { // because the sweeper thread will just try again after its next sleep (max sleep is 10 seconds) return; } - synchronized(cache.getTombstoneService().blockGCLock) { - int count = cache.getTombstoneService().getGCBlockCount(); + synchronized(getBlockGCLock()) { + int count = getGCBlockCount(); if (count > 0) { // if any delta GII is on going as provider at this member, not to do tombstone GC if (logger.isDebugEnabled()) { @@ -615,9 +639,10 @@ public class TombstoneService { @Override boolean forceBatchExpirationForTests(int count) throws InterruptedException { - // TODO: shouldn't this method make sure the sweeper is not currently doing - // batch expire? If it is then the latch will get counted down early. - testHook_forceBatchExpireCall = new CountDownLatch(1); + // sync on blockGCLock since expireBatch syncs on it + synchronized(getBlockGCLock()) { + testHook_forceBatchExpireCall = new CountDownLatch(1); + } try { synchronized(this) { testHook_forceExpirationCount += count;
