You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2016/07/05 22:05:18 UTC
[8/9] incubator-geode git commit: the expiredTombstones collection is
now an ArrayList and is final
the expiredTombstones collection is now an ArrayList and is final
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/850adfc6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/850adfc6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/850adfc6
Branch: refs/heads/develop
Commit: 850adfc653ece2f1f3e54f6f6d208acaf6aad142
Parents: 6e2a9b2
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Fri Jun 24 17:20:49 2016 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Tue Jul 5 14:30:10 2016 -0700
----------------------------------------------------------------------
.../internal/cache/TombstoneService.java | 82 ++++----------------
.../DistributedAckRegionCCEDUnitTest.java | 2 +-
.../gemfire/cache30/MultiVMRegionTestCase.java | 2 +-
.../PersistentRVVRecoveryDUnitTest.java | 3 +-
4 files changed, 20 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/850adfc6/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 6989ed2..e6dcfac 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
@@ -80,7 +80,7 @@ public class TombstoneService {
* all replicated regions, including PR buckets. The default is
* 100,000 expired tombstones.
*/
- public static long EXPIRED_TOMBSTONE_LIMIT = Long.getLong(DistributionConfig.GEMFIRE_PREFIX + "tombstone-gc-threshold", 100000);
+ public static int EXPIRED_TOMBSTONE_LIMIT = Integer.getInteger(DistributionConfig.GEMFIRE_PREFIX + "tombstone-gc-threshold", 100000);
/**
* The interval to scan for expired tombstones in the queues
@@ -371,31 +371,6 @@ public class TombstoneService {
}
}
- /**
- * Test Hook - slow operation
- * verify whether a tombstone is scheduled for expiration
- */
- public boolean isTombstoneScheduled(LocalRegion r, RegionEntry re) {
- TombstoneSweeper sweeper = this.getSweeper(r);
- VersionSource myId = r.getVersionMember();
- VersionTag entryTag = re.getVersionStamp().asVersionTag();
- int entryVersion = entryTag.getEntryVersion();
- for (Tombstone t: sweeper.getQueue()) {
- if (t.region == r) {
- VersionSource destroyingMember = t.getMemberID();
- if (destroyingMember == null) {
- destroyingMember = myId;
- }
- if (t.region == r
- && t.entry.getKey().equals(re.getKey())
- && t.getEntryVersion() == entryVersion) {
- return true;
- }
- }
- }
- return sweeper.hasExpiredTombstone(r, re, entryTag);
- }
-
@Override
public String toString() {
return "Destroyed entries GC service. Replicate Queue=" + this.replicatedTombstoneSweeper.getQueue().toString()
@@ -474,7 +449,7 @@ public class TombstoneService {
* tombstones that have expired and are awaiting batch removal. This
* variable is only accessed by the sweeper thread and so is not guarded
*/
- private Set<Tombstone> expiredTombstones;
+ private final List<Tombstone> expiredTombstones;
/**
* count of entries to forcibly expire due to memory events
@@ -488,6 +463,8 @@ public class TombstoneService {
/**
* Is a batch expiration in progress?
+ * Part of expireBatch is done in a background thread
+ * and until that completes batch expiration is in progress.
*/
private volatile boolean batchExpirationInProgress;
@@ -515,7 +492,9 @@ public class TombstoneService {
this.queueSize = queueSize;
this.batchMode = batchMode;
if (batchMode) {
- this.expiredTombstones = new HashSet<Tombstone>();
+ this.expiredTombstones = new ArrayList<Tombstone>();
+ } else {
+ this.expiredTombstones = null;
}
this.currentTombstoneLock = new StoppableReentrantLock(cache.getCancelCriterion());
this.sweeperThread = new Thread(LoggingThreadGroup.createThreadGroup("Destroyed Entries Processors", logger), this);
@@ -583,32 +562,6 @@ public class TombstoneService {
}
}
- /** test hook - unsafe since not synchronized */
- boolean hasExpiredTombstone(LocalRegion r, RegionEntry re, VersionTag tag) {
- if (this.expiredTombstones == null) {
- return false;
- }
- int entryVersion = tag.getEntryVersion();
- boolean retry;
- do {
- retry = false;
- try {
- for (Tombstone t: this.expiredTombstones) {
- if (t.region == r
- && t.entry.getKey().equals(re.getKey())
- && t.getEntryVersion() == entryVersion) {
- return true;
- }
- }
- } catch (ConcurrentModificationException e) {
- retry = true;
- }
- } while (retry);
- return false;
- }
-
-
-
/** expire a batch of tombstones */
private void expireBatch() {
// fix for bug #46087 - OOME due to too many GC threads
@@ -630,11 +583,6 @@ public class TombstoneService {
this.batchExpirationInProgress = true;
boolean batchScheduled = false;
try {
- Set<Tombstone> expired = expiredTombstones;
- if (expired.isEmpty()) {
- return;
- }
- expiredTombstones = new HashSet<Tombstone>();
long removalSize = 0;
{
@@ -642,7 +590,7 @@ public class TombstoneService {
//Update the GC RVV for all of the affected regions.
//We need to do this so that we can persist the GC RVV before
//we start removing entries from the map.
- for (Tombstone t: expired) {
+ for (Tombstone t: expiredTombstones) {
t.region.getVersionVector().recordGCVersion(t.getMemberID(), t.getRegionVersion());
regionsAffected.add((DistributedRegion)t.region);
}
@@ -661,10 +609,15 @@ public class TombstoneService {
}
}
+ // TODO seems like no need for the value of this map to be a Set.
+ // It could instead be a List, which would be nice because the per entry
+ // memory overhead for a set is much higher than an ArrayList
+ // BUT we send it to clients and the old
+ // version of them expects it to be a Set.
final Map<DistributedRegion, Set<Object>> reapedKeys = new HashMap<>();
//Remove the tombstones from the in memory region map.
- for (Tombstone t: expired) {
+ for (Tombstone t: expiredTombstones) {
// for PR buckets we have to keep track of the keys removed because clients have
// them all lumped in a single non-PR region
DistributedRegion tr = (DistributedRegion) t.region;
@@ -679,6 +632,7 @@ public class TombstoneService {
}
removalSize += t.getSize();
}
+ expiredTombstones.clear();
this.queueSize.addAndGet(-removalSize);
if (!reapedKeys.isEmpty()) {
@@ -896,10 +850,8 @@ public class TombstoneService {
}
// test hook: if there are expired tombstones and nothing else is expiring soon,
// perform distributed tombstone GC
- if (batchMode && IDLE_EXPIRATION && sleepTime >= expiryTime) {
- if (this.expiredTombstones.size() > 0) {
- expireBatch();
- }
+ if (batchMode && IDLE_EXPIRATION && sleepTime >= expiryTime && !this.expiredTombstones.isEmpty()) {
+ expireBatch();
}
if (sleepTime > 0) {
try {
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/850adfc6/geode-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionCCEDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionCCEDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionCCEDUnitTest.java
index 652bd6b..1aabbb5 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionCCEDUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionCCEDUnitTest.java
@@ -368,7 +368,7 @@ public class DistributedAckRegionCCEDUnitTest extends DistributedAckRegionDUnitT
final String name = this.getUniqueName() + "-CC";
- final long saveExpiredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
+ final int saveExpiredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
final long saveTombstoneTimeout = TombstoneService.REPLICATED_TOMBSTONE_TIMEOUT;
TombstoneService.EXPIRED_TOMBSTONE_LIMIT = 50;
TombstoneService.REPLICATED_TOMBSTONE_TIMEOUT = 500;
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/850adfc6/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java b/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
index f5c6c03..ac2fdb0 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
@@ -8554,7 +8554,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
// sure that all three regions are consistent
final long oldServerTimeout = TombstoneService.REPLICATED_TOMBSTONE_TIMEOUT;
final long oldClientTimeout = TombstoneService.CLIENT_TOMBSTONE_TIMEOUT;
- final long oldExpiredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
+ final int oldExpiredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
final boolean oldIdleExpiration = TombstoneService.IDLE_EXPIRATION;
final double oldLimit = TombstoneService.GC_MEMORY_THRESHOLD;
try {
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/850adfc6/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/persistence/PersistentRVVRecoveryDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/persistence/PersistentRVVRecoveryDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/persistence/PersistentRVVRecoveryDUnitTest.java
index f7c011d..0a2e673 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/persistence/PersistentRVVRecoveryDUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/persistence/PersistentRVVRecoveryDUnitTest.java
@@ -266,9 +266,8 @@ public class PersistentRVVRecoveryDUnitTest extends PersistentReplicatedTestBase
@Override
public void run2() throws CacheException {
- // TODO Auto-generated method stub
long replicatedTombstoneTomeout = TombstoneService.REPLICATED_TOMBSTONE_TIMEOUT;
- long expiriredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
+ int expiriredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
try {
LocalRegion region = createRegion(vm0);