You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jc...@apache.org on 2020/04/02 19:45:54 UTC

[geode] 01/01: Replace Map with ConcurrentMap for GemFireCacheImpl.rootRegions

This is an automated email from the ASF dual-hosted git repository.

jchen21 pushed a commit to branch feature/GEODE-7945
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 020ea33054dfd09dfed12a6d8b7be2ca9d9645a7
Author: Jianxia Chen <jc...@apache.org>
AuthorDate: Thu Apr 2 12:44:21 2020 -0700

    Replace Map with ConcurrentMap for GemFireCacheImpl.rootRegions
    
    Authored-by: Jianxia Chen <jc...@apache.org>
---
 .../geode/internal/cache/GemFireCacheImpl.java     | 242 ++++++++++-----------
 1 file changed, 120 insertions(+), 122 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index 93b8e0e..37177f8 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -408,7 +408,7 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
 
   private final DistributionManager dm;
 
-  private final Map<String, InternalRegion> rootRegions;
+  private final ConcurrentMap<String, InternalRegion> rootRegions;
 
   /**
    * True if this cache is being created by a ClientCacheFactory.
@@ -998,7 +998,7 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
         throw new IllegalStateException("Cannot create a Cache in an admin-only VM.");
       }
 
-      rootRegions = new HashMap<>();
+      rootRegions = new ConcurrentHashMap<>();
 
       cqService = cqServiceFactory.apply(this);
 
@@ -2176,9 +2176,8 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
           prepareDiskStoresForClose();
 
           List<InternalRegion> rootRegionValues;
-          synchronized (rootRegions) {
-            rootRegionValues = new ArrayList<>(rootRegions.values());
-          }
+
+          rootRegionValues = new ArrayList<>(rootRegions.values());
 
           Operation op;
           if (forcedDisconnect) {
@@ -2905,45 +2904,44 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
         getCancelCriterion().checkCancelInProgress(null);
 
         Future<InternalRegion> future = null;
-        synchronized (rootRegions) {
-          region = rootRegions.get(name);
-          if (region != null) {
-            throw new RegionExistsException(region);
-          }
-          // check for case where a root region is being reinitialized
-          // and we didn't find a region, i.e. the new region is about to be created
 
-          if (!isReInitCreate) {
-            String fullPath = Region.SEPARATOR + name;
-            future = reinitializingRegions.get(fullPath);
-          }
-          if (future == null) {
-            if (internalRegionArgs.getInternalMetaRegion() != null) {
-              region = internalRegionArgs.getInternalMetaRegion();
-            } else if (isPartitionedRegion) {
-              region = new PartitionedRegion(name, attrs, null, this, internalRegionArgs,
-                  statisticsClock, ColocationLoggerFactory.create());
+        region = rootRegions.get(name);
+        if (region != null) {
+          throw new RegionExistsException(region);
+        }
+        // check for case where a root region is being reinitialized
+        // and we didn't find a region, i.e. the new region is about to be created
+
+        if (!isReInitCreate) {
+          String fullPath = Region.SEPARATOR + name;
+          future = reinitializingRegions.get(fullPath);
+        }
+        if (future == null) {
+          if (internalRegionArgs.getInternalMetaRegion() != null) {
+            region = internalRegionArgs.getInternalMetaRegion();
+          } else if (isPartitionedRegion) {
+            region = new PartitionedRegion(name, attrs, null, this, internalRegionArgs,
+                statisticsClock, ColocationLoggerFactory.create());
+          } else {
+            // Abstract region depends on the default pool existing so lazily initialize it
+            // if necessary.
+            if (Objects.equals(attrs.getPoolName(), DEFAULT_POOL_NAME)) {
+              determineDefaultPool();
+            }
+            if (attrs.getScope().isLocal()) {
+              region =
+                  new LocalRegion(name, attrs, null, this, internalRegionArgs, statisticsClock);
             } else {
-              // Abstract region depends on the default pool existing so lazily initialize it
-              // if necessary.
-              if (Objects.equals(attrs.getPoolName(), DEFAULT_POOL_NAME)) {
-                determineDefaultPool();
-              }
-              if (attrs.getScope().isLocal()) {
-                region =
-                    new LocalRegion(name, attrs, null, this, internalRegionArgs, statisticsClock);
-              } else {
-                region = new DistributedRegion(name, attrs, null, this, internalRegionArgs,
-                    statisticsClock);
-              }
+              region = new DistributedRegion(name, attrs, null, this, internalRegionArgs,
+                  statisticsClock);
             }
+          }
 
-            rootRegions.put(name, region);
-            if (isReInitCreate) {
-              regionReinitialized(region);
-            }
-            break;
+          rootRegions.put(name, region);
+          if (isReInitCreate) {
+            regionReinitialized(region);
           }
+          break;
         }
 
         boolean interrupted = Thread.interrupted();
@@ -2997,12 +2995,12 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
           } finally {
             // clean up if initialize fails for any reason
             setRegionByPath(region.getFullPath(), null);
-            synchronized (rootRegions) {
-              Region rootRegion = rootRegions.get(name);
-              if (rootRegion == region) {
-                rootRegions.remove(name);
-              }
+
+            Region rootRegion = rootRegions.get(name);
+            if (rootRegion == region) {
+              rootRegions.remove(name);
             }
+
           }
         }
       }
@@ -3064,60 +3062,60 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
   @Override
   public Set<InternalRegion> getAllRegions() {
     Set<InternalRegion> result = new HashSet<>();
-    synchronized (rootRegions) {
-      for (Region<?, ?> region : rootRegions.values()) {
-        if (region instanceof PartitionedRegion) {
-          PartitionedRegion partitionedRegion = (PartitionedRegion) region;
-          PartitionedRegionDataStore dataStore = partitionedRegion.getDataStore();
-          if (dataStore != null) {
-            Set<Entry<Integer, BucketRegion>> bucketEntries =
-                partitionedRegion.getDataStore().getAllLocalBuckets();
-            for (Entry entry : bucketEntries) {
-              result.add((InternalRegion) entry.getValue());
-            }
+
+    for (Region<?, ?> region : rootRegions.values()) {
+      if (region instanceof PartitionedRegion) {
+        PartitionedRegion partitionedRegion = (PartitionedRegion) region;
+        PartitionedRegionDataStore dataStore = partitionedRegion.getDataStore();
+        if (dataStore != null) {
+          Set<Entry<Integer, BucketRegion>> bucketEntries =
+              partitionedRegion.getDataStore().getAllLocalBuckets();
+          for (Entry entry : bucketEntries) {
+            result.add((InternalRegion) entry.getValue());
           }
-        } else if (region instanceof InternalRegion) {
-          InternalRegion internalRegion = (InternalRegion) region;
-          result.add(internalRegion);
-          result.addAll(internalRegion.basicSubregions(true));
         }
+      } else if (region instanceof InternalRegion) {
+        InternalRegion internalRegion = (InternalRegion) region;
+        result.add(internalRegion);
+        result.addAll(internalRegion.basicSubregions(true));
       }
     }
+
     return result;
   }
 
   @Override
   public Set<InternalRegion> getApplicationRegions() {
     Set<InternalRegion> result = new HashSet<>();
-    synchronized (rootRegions) {
-      for (Object region : rootRegions.values()) {
-        InternalRegion internalRegion = (InternalRegion) region;
-        if (internalRegion.isInternalRegion()) {
-          // Skip internal regions
-          continue;
-        }
-        result.add(internalRegion);
-        result.addAll(internalRegion.basicSubregions(true));
+
+    for (Object region : rootRegions.values()) {
+      InternalRegion internalRegion = (InternalRegion) region;
+      if (internalRegion.isInternalRegion()) {
+        // Skip internal regions
+        continue;
       }
+      result.add(internalRegion);
+      result.addAll(internalRegion.basicSubregions(true));
     }
+
     return result;
   }
 
   @Override
   public boolean hasPersistentRegion() {
-    synchronized (rootRegions) {
-      for (InternalRegion region : rootRegions.values()) {
-        if (region.getDataPolicy().withPersistence()) {
+
+    for (InternalRegion region : rootRegions.values()) {
+      if (region.getDataPolicy().withPersistence()) {
+        return true;
+      }
+      for (InternalRegion subRegion : region.basicSubregions(true)) {
+        if (subRegion.getDataPolicy().withPersistence()) {
           return true;
         }
-        for (InternalRegion subRegion : region.basicSubregions(true)) {
-          if (subRegion.getDataPolicy().withPersistence()) {
-            return true;
-          }
-        }
       }
-      return false;
     }
+    return false;
+
   }
 
   @Override
@@ -3172,12 +3170,12 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
       try {
         String[] pathParts = parsePath(path);
         InternalRegion rootRegion;
-        synchronized (rootRegions) {
-          rootRegion = rootRegions.get(pathParts[0]);
-          if (rootRegion == null) {
-            return null;
-          }
+
+        rootRegion = rootRegions.get(pathParts[0]);
+        if (rootRegion == null) {
+          return null;
         }
+
         if (logger.isDebugEnabled()) {
           logger.debug("GemFireCache.getRegion, calling getSubregion on rootRegion({}): {}",
               pathParts[0], pathParts[1]);
@@ -3208,21 +3206,21 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
 
     String[] pathParts = parsePath(path);
     InternalRegion rootRegion;
-    synchronized (rootRegions) {
-      rootRegion = rootRegions.get(pathParts[0]);
-      if (rootRegion == null) {
-        if (logger.isDebugEnabled()) {
-          logger.debug("GemFireCache.getRegion, no region found for {}", pathParts[0]);
-        }
-        stopper.checkCancelInProgress(null);
-        return null;
-      }
-      if (!returnDestroyedRegion && rootRegion.isDestroyed()) {
-        stopper.checkCancelInProgress(null);
-        return null;
+
+    rootRegion = rootRegions.get(pathParts[0]);
+    if (rootRegion == null) {
+      if (logger.isDebugEnabled()) {
+        logger.debug("GemFireCache.getRegion, no region found for {}", pathParts[0]);
       }
+      stopper.checkCancelInProgress(null);
+      return null;
+    }
+    if (!returnDestroyedRegion && rootRegion.isDestroyed()) {
+      stopper.checkCancelInProgress(null);
+      return null;
     }
 
+
     if (logger.isDebugEnabled()) {
       logger.debug("GemFireCache.getRegion, calling getSubregion on rootRegion({}): {}",
           pathParts[0], pathParts[1]);
@@ -3268,20 +3266,20 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
   private Set<Region<?, ?>> rootRegions(boolean includePRAdminRegions, boolean waitForInit) {
     stopper.checkCancelInProgress(null);
     Set<Region<?, ?>> regions = new HashSet<>();
-    synchronized (rootRegions) {
-      for (InternalRegion region : rootRegions.values()) {
-        // If this is an internal meta-region, don't return it to end user
-        if (region.isSecret()
-            || region.isUsedForMetaRegion()
-            || !includePRAdminRegions
-                && (region.isUsedForPartitionedRegionAdmin()
-                    || region.isUsedForPartitionedRegionBucket())) {
-          // Skip administrative PartitionedRegions
-          continue;
-        }
-        regions.add(region);
+
+    for (InternalRegion region : rootRegions.values()) {
+      // If this is an internal meta-region, don't return it to end user
+      if (region.isSecret()
+          || region.isUsedForMetaRegion()
+          || !includePRAdminRegions
+              && (region.isUsedForPartitionedRegionAdmin()
+                  || region.isUsedForPartitionedRegionBucket())) {
+        // Skip administrative PartitionedRegions
+        continue;
       }
+      regions.add(region);
     }
+
     if (waitForInit) {
       for (Iterator<Region<?, ?>> iterator = regions.iterator(); iterator.hasNext();) {
         InternalRegion region = (InternalRegion) iterator.next();
@@ -3438,16 +3436,16 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
 
   @Override
   public boolean removeRoot(InternalRegion rootRgn) {
-    synchronized (rootRegions) {
-      String regionName = rootRgn.getName();
-      InternalRegion found = rootRegions.get(regionName);
-      if (found == rootRgn) {
-        InternalRegion previous = rootRegions.remove(regionName);
-        Assert.assertTrue(previous == rootRgn);
-        return true;
-      }
-      return false;
+
+    String regionName = rootRgn.getName();
+    InternalRegion found = rootRegions.get(regionName);
+    if (found == rootRgn) {
+      InternalRegion previous = rootRegions.remove(regionName);
+      Assert.assertTrue(previous == rootRgn);
+      return true;
     }
+    return false;
+
   }
 
   /**
@@ -3621,16 +3619,16 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
       }
     }
 
-    synchronized (rootRegions) {
-      Set<InternalRegion> applicationRegions = getApplicationRegions();
-      for (InternalRegion region : applicationRegions) {
-        Set<String> senders = region.getAllGatewaySenderIds();
-        if (senders.contains(sender.getId()) && !sender.isParallel()) {
-          region.senderCreated();
-        }
+
+    Set<InternalRegion> applicationRegions = getApplicationRegions();
+    for (InternalRegion region : applicationRegions) {
+      Set<String> senders = region.getAllGatewaySenderIds();
+      if (senders.contains(sender.getId()) && !sender.isParallel()) {
+        region.senderCreated();
       }
     }
 
+
     if (!sender.isParallel()) {
       Region<?, ?> dynamicMetaRegion = getRegion(DynamicRegionFactory.DYNAMIC_REGION_LIST_NAME);
       if (dynamicMetaRegion == null) {