You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2017/03/17 20:10:52 UTC

[17/49] geode git commit: GEODE-2616: fix leak in colocated region removal

GEODE-2616: fix leak in colocated region removal

postDestroyRegion now removes itself from colocated parent colocatedByList


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/72378a74
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/72378a74
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/72378a74

Branch: refs/heads/feature/GEODE-2420
Commit: 72378a743471ec53bffeef9cfb92f1c98fdd19bb
Parents: c6c6459
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Fri Jan 20 16:05:41 2017 -0800
Committer: Ken Howe <kh...@pivotal.io>
Committed: Fri Mar 17 13:09:44 2017 -0700

----------------------------------------------------------------------
 .../cache/query/internal/DefaultQuery.java      |  4 +-
 .../geode/internal/cache/PartitionedRegion.java | 12 ++---
 .../internal/cache/ColocatedPRJUnitTest.java    | 47 ++++++++++++++++++++
 .../cache/PartitionedRegionTestHelper.java      | 11 ++++-
 4 files changed, 65 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/72378a74/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
index 8bfd4fa..a721091 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
@@ -650,8 +650,8 @@ public class DefaultQuery implements Query {
             continue;
           }
           other = allPRs;
-          if ((((PartitionedRegion) eachPR).colocatedByList.contains(allPRs)
-              || ((PartitionedRegion) allPRs).colocatedByList.contains(eachPR))) {
+          if ((((PartitionedRegion) eachPR).getColocatedByList().contains(allPRs)
+              || ((PartitionedRegion) allPRs).getColocatedByList().contains(eachPR))) {
             colocated = true;
             break;
           }

http://git-wip-us.apache.org/repos/asf/geode/blob/72378a74/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
index 9374d4b..173f35c 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
@@ -600,7 +600,8 @@ public class PartitionedRegion extends LocalRegion
 
   private byte fixedPASet;
 
-  public List<PartitionedRegion> colocatedByList = new CopyOnWriteArrayList<PartitionedRegion>();
+  private final List<PartitionedRegion> colocatedByList =
+      new CopyOnWriteArrayList<PartitionedRegion>();
 
   private final PartitionListener[] partitionListeners;
 
@@ -670,9 +671,7 @@ public class PartitionedRegion extends LocalRegion
     this.colocatedWithRegion = ColocationHelper.getColocatedRegion(this);
 
     if (colocatedWithRegion != null) {
-      synchronized (colocatedWithRegion.colocatedByList) {
-        colocatedWithRegion.colocatedByList.add(this);
-      }
+      colocatedWithRegion.getColocatedByList().add(this);
     }
 
     if (colocatedWithRegion != null && !internalRegionArgs.isUsedForParallelGatewaySenderQueue()) {
@@ -786,7 +785,7 @@ public class PartitionedRegion extends LocalRegion
     return parallelGatewaySenderIds;
   }
 
-  List<PartitionedRegion> getColocatedByList() {
+  public List<PartitionedRegion> getColocatedByList() {
     return this.colocatedByList;
   }
 
@@ -7922,6 +7921,9 @@ public class PartitionedRegion extends LocalRegion
         }
       }
     }
+    if (colocatedWithRegion != null) {
+      colocatedWithRegion.getColocatedByList().remove(this);
+    }
 
     RegionLogger.logDestroy(getName(), cache.getMyId(), null, op.isClose());
   }

http://git-wip-us.apache.org/repos/asf/geode/blob/72378a74/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java
new file mode 100644
index 0000000..66484d9
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class ColocatedPRJUnitTest {
+  @SuppressWarnings("rawtypes")
+  @Test
+  public void destroyColocatedPRCheckForLeak() {
+    PartitionedRegion parent =
+        (PartitionedRegion) PartitionedRegionTestHelper.createPartionedRegion("PARENT");
+    List<PartitionedRegion> colocatedList = parent.getColocatedByList();
+    assertEquals(0, colocatedList.size());
+    PartitionAttributes PRatts =
+        new PartitionAttributesFactory().setColocatedWith("/PARENT").create();
+    PartitionedRegion child =
+        (PartitionedRegion) PartitionedRegionTestHelper.createPartionedRegion("CHILD", PRatts);
+    assertTrue(colocatedList.contains(child));
+    child.destroyRegion();
+    assertFalse(colocatedList.contains(child));
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/72378a74/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java
index 42c48ac..2824d74 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java
@@ -49,9 +49,16 @@ public class PartitionedRegionTestHelper
    */
 
   public static Region createPartionedRegion(String regionname) throws RegionExistsException {
+    return createPartionedRegion(regionname, new PartitionAttributesFactory().create());
+  }
+
+  /**
+   * This method creates a partitioned region with the given PR attributes. The cache created is a
+   * loner, so this is only suitable for single VM tests.
+   */
+  public static Region createPartionedRegion(String regionname, PartitionAttributes prattribs)
+      throws RegionExistsException {
     AttributesFactory attribFactory = new AttributesFactory();
-    PartitionAttributesFactory paf = new PartitionAttributesFactory();
-    PartitionAttributes prattribs = paf.create();
     attribFactory.setDataPolicy(DataPolicy.PARTITION);
     attribFactory.setPartitionAttributes(prattribs);
     RegionAttributes regionAttribs = attribFactory.create();