albertogpz commented on code in PR #7857:
URL: https://github.com/apache/geode/pull/7857#discussion_r973919611


##########
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java:
##########
@@ -483,30 +485,73 @@ public void testColocationEnforceLocalMaxMemory() throws 
UnknownHostException {
   }
 
   /**
-   * Test that each region individually honors it's enforce local max memory 
flag.
+   * Test that a region with enforceLocalMaxMemory disabled colocated with
+   * a region that memory full with enforceLocalMaxMemory enabled will prevent 
a bucket move.
    */
   @Test
-  public void testColocationIgnoreEnforceLocalMaxMemory() throws 
UnknownHostException {
+  public void testColocationOneNoneEvictionRegionReachLocalMaxMemoryLimit() 
throws UnknownHostException {

Review Comment:
   I would rename the test case to 
`testColocationOneNonEvictionRegionReachesLocalMaxMemoryLimit()`



##########
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java:
##########
@@ -483,30 +485,73 @@ public void testColocationEnforceLocalMaxMemory() throws 
UnknownHostException {
   }
 
   /**
-   * Test that each region individually honors it's enforce local max memory 
flag.
+   * Test that a region with enforceLocalMaxMemory disabled colocated with
+   * a region that memory full with enforceLocalMaxMemory enabled will prevent 
a bucket move.
    */
   @Test
-  public void testColocationIgnoreEnforceLocalMaxMemory() throws 
UnknownHostException {
+  public void testColocationOneNoneEvictionRegionReachLocalMaxMemoryLimit() 
throws UnknownHostException {
     PartitionedRegionLoadModel model = new 
PartitionedRegionLoadModel(bucketOperator, 1, 4,
         getAddressComparor(false), Collections.emptySet(), partitionedRegion);
     InternalDistributedMember member1 =
         new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 1);
     InternalDistributedMember member2 =
         new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 2);
 
-    // Create some buckets with low redundancy on member 1
     PartitionMemberInfoImpl details1 =
-        buildDetails(member1, new long[] {1, 1, 1, 1}, new long[] {1, 1, 1, 
1});
+        buildDetails(member1, 1, 8 * MB, new long[] {1 * MB, 1 * MB, 1 * MB, 1 
* MB},
+            new long[] {1, 1, 1, 1});
     PartitionMemberInfoImpl details2 =
-        buildDetails(member2, new long[] {0, 0, 0, 0}, new long[] {0, 0, 0, 
0});
+        buildDetails(member2, 1, 8 * MB, new long[] {0, 0, 0, 0}, new long[] 
{0, 0, 0, 0});
+    model.addRegion("a", Arrays.asList(details1, details2), new 
FakeOfflineDetails(), false);
+
+
+    PartitionMemberInfoImpl bDetails1 =
+        buildDetails(member1, 1, 2 * MB, new long[] {1 * MB, 1 * MB, 1 * MB, 1 
* MB},
+            new long[] {1, 1, 1, 1});
+    PartitionMemberInfoImpl bDetails2 =
+        buildDetails(member2, 1, 2 * MB, new long[] {0, 0, 0, 0}, new long[] 
{0, 0, 0, 0});
+    model.addRegion("b", Arrays.asList(bDetails1, bDetails2), new 
FakeOfflineDetails(), true);
+
+
+    assertThat(doMoves(new CompositeDirector(true, true, false, true), 
model)).isEqualTo(4);
+
+    Set<Create> expectedCreates = new HashSet<>();
+    expectedCreates.add(new Create(member2, 0));
+    expectedCreates.add(new Create(member2, 1));
+    assertThat(new 
HashSet<>(bucketOperator.creates)).isEqualTo(expectedCreates);
+
+    Set<Move> expectedMoves = new HashSet<>();
+    expectedMoves.add(new Move(member1, member2));
+    expectedMoves.add(new Move(member1, member2));
+    assertThat(new 
HashSet<>(bucketOperator.primaryMoves)).isEqualTo(expectedMoves);
+  }
+
+  /**
+   * Test that a region that memory full with enforceLocalMaxMemory disabled 
will not prevent a
+   * bucket move.
+   */
+  @Test
+  public void testColocationOneEvictionRegionReachLocalMaxMemoryLimit() throws 
UnknownHostException {

Review Comment:
   I would rename the test case to: 
`testColocationOneEvictionRegionReachesLocalMaxMemoryLimit()`



##########
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java:
##########
@@ -483,30 +485,73 @@ public void testColocationEnforceLocalMaxMemory() throws 
UnknownHostException {
   }
 
   /**
-   * Test that each region individually honors it's enforce local max memory 
flag.
+   * Test that a region with enforceLocalMaxMemory disabled colocated with
+   * a region that memory full with enforceLocalMaxMemory enabled will prevent 
a bucket move.
    */
   @Test
-  public void testColocationIgnoreEnforceLocalMaxMemory() throws 
UnknownHostException {
+  public void testColocationOneNoneEvictionRegionReachLocalMaxMemoryLimit() 
throws UnknownHostException {
     PartitionedRegionLoadModel model = new 
PartitionedRegionLoadModel(bucketOperator, 1, 4,
         getAddressComparor(false), Collections.emptySet(), partitionedRegion);
     InternalDistributedMember member1 =
         new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 1);
     InternalDistributedMember member2 =
         new InternalDistributedMember(InetAddress.getByName("127.0.0.1"), 2);
 
-    // Create some buckets with low redundancy on member 1
     PartitionMemberInfoImpl details1 =
-        buildDetails(member1, new long[] {1, 1, 1, 1}, new long[] {1, 1, 1, 
1});
+        buildDetails(member1, 1, 8 * MB, new long[] {1 * MB, 1 * MB, 1 * MB, 1 
* MB},
+            new long[] {1, 1, 1, 1});
     PartitionMemberInfoImpl details2 =
-        buildDetails(member2, new long[] {0, 0, 0, 0}, new long[] {0, 0, 0, 
0});
+        buildDetails(member2, 1, 8 * MB, new long[] {0, 0, 0, 0}, new long[] 
{0, 0, 0, 0});
+    model.addRegion("a", Arrays.asList(details1, details2), new 
FakeOfflineDetails(), false);
+
+
+    PartitionMemberInfoImpl bDetails1 =
+        buildDetails(member1, 1, 2 * MB, new long[] {1 * MB, 1 * MB, 1 * MB, 1 
* MB},
+            new long[] {1, 1, 1, 1});
+    PartitionMemberInfoImpl bDetails2 =
+        buildDetails(member2, 1, 2 * MB, new long[] {0, 0, 0, 0}, new long[] 
{0, 0, 0, 0});
+    model.addRegion("b", Arrays.asList(bDetails1, bDetails2), new 
FakeOfflineDetails(), true);
+
+
+    assertThat(doMoves(new CompositeDirector(true, true, false, true), 
model)).isEqualTo(4);
+
+    Set<Create> expectedCreates = new HashSet<>();
+    expectedCreates.add(new Create(member2, 0));
+    expectedCreates.add(new Create(member2, 1));
+    assertThat(new 
HashSet<>(bucketOperator.creates)).isEqualTo(expectedCreates);
+
+    Set<Move> expectedMoves = new HashSet<>();
+    expectedMoves.add(new Move(member1, member2));
+    expectedMoves.add(new Move(member1, member2));
+    assertThat(new 
HashSet<>(bucketOperator.primaryMoves)).isEqualTo(expectedMoves);
+  }
+
+  /**
+   * Test that a region that memory full with enforceLocalMaxMemory disabled 
will not prevent a

Review Comment:
   I would change this to: Test that a region with memory full and 
enforceLocalMaxMemory...



##########
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java:
##########
@@ -443,7 +443,7 @@ public void testIncompleteColocation() throws 
UnknownHostException {
    * lmm, it will prevent a bucket move
    */
   @Test
-  public void testColocationEnforceLocalMaxMemory() throws 
UnknownHostException {
+  public void testColocationTwoNoneEvictionRegions() throws 
UnknownHostException {

Review Comment:
   I would rename the test case to: 
`testColocationTwoNonEvictionRegionsEnforceLocalMaxMemory()`



##########
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java:
##########
@@ -483,30 +485,73 @@ public void testColocationEnforceLocalMaxMemory() throws 
UnknownHostException {
   }
 
   /**
-   * Test that each region individually honors it's enforce local max memory 
flag.
+   * Test that a region with enforceLocalMaxMemory disabled colocated with
+   * a region that memory full with enforceLocalMaxMemory enabled will prevent 
a bucket move.

Review Comment:
   I would change this sentence to: a region with memory full and 
enforceLocalmaxMemory enabled...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to