geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [8/9] incubator-geode git commit: the expiredTombstones collection is now an ArrayList and is final
Date Tue, 05 Jul 2016 22:05:18 GMT
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 <dschneider@pivotal.io>
Authored: Fri Jun 24 17:20:49 2016 -0700
Committer: Darrel Schneider <dschneider@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);


Mime
View raw message