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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
