You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/06/01 18:15:26 UTC
[GitHub] [geode] DonalEvans commented on a diff in pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
DonalEvans commented on code in PR #7744:
URL: https://github.com/apache/geode/pull/7744#discussion_r887158208
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionStatsDistributedTest.java:
##########
@@ -51,55 +51,67 @@ public class PartitionedRegionStatsDUnitTest extends CacheTestCase {
public static final String STAT_CONFIGURED_REDUNDANT_COPIES = "configuredRedundantCopies";
public static final String STAT_ACTUAL_REDUNDANT_COPIES = "actualRedundantCopies";
- private VM vm0;
- private VM vm1;
- private VM vm2;
- private VM vm3;
-
- private String regionName;
+ @Rule
+ public ClusterStartupRule clusterStartupRule = new ClusterStartupRule();
- @Before
- public void setUp() throws Exception {
- vm0 = getHost(0).getVM(0);
- vm1 = getHost(0).getVM(1);
- vm2 = getHost(0).getVM(2);
- vm3 = getHost(0).getVM(3);
-
- regionName = "region";
- }
+ private final String regionName = "region";
@Test
- public void testClose() throws Exception {
+ public void whenOperationsAreDoneOnEntriesThenStatsMustBeCorrect() {
+ MemberVM locator1 = clusterStartupRule.startLocatorVM(0);
+ int locator1Port = locator1.getPort();
+ MemberVM locator2 =
+ clusterStartupRule.startLocatorVM(1, l -> l.withConnectionToLocator(locator1Port));
+ int locator2Port = locator2.getPort();
+
+ MemberVM server1 =
+ clusterStartupRule.startServerVM(2,
+ s -> s.withConnectionToLocator(locator1Port, locator2Port));
+ MemberVM server2 =
+ clusterStartupRule.startServerVM(3,
+ s -> s.withConnectionToLocator(locator1Port, locator2Port));
+ MemberVM server3 =
+ clusterStartupRule.startServerVM(4,
+ s -> s.withConnectionToLocator(locator1Port, locator2Port));
+ MemberVM server4 =
+ clusterStartupRule.startServerVM(5,
+ s -> s.withConnectionToLocator(locator1Port, locator2Port));
+
// Create PRs on 3 VMs and accessors on 1 VM
- vm0.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
- vm1.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
- vm2.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
- vm3.invoke(() -> createPartitionedRegion(regionName, 0, 1, 5));
+ server1.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
+ server2.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
+ server3.invoke(() -> createPartitionedRegion(regionName, 200, 1, 5));
+ server4.invoke(() -> createPartitionedRegion(regionName, 0, 1, 5));
// Do Region operations.
- vm0.invoke(() -> doOpsOnRegion(regionName, PUT));
- vm0.invoke(() -> doOpsOnRegion(regionName, GET));
- vm0.invoke(() -> doOpsOnRegion(regionName, GET_ENTRY));
- vm0.invoke(() -> doOpsOnRegion(regionName, CONTAINS_KEY));
- vm0.invoke(() -> doOpsOnRegion(regionName, CONTAINS_VALUE_FOR_KEY));
- vm0.invoke(() -> doOpsOnRegion(regionName, INVALIDATE));
- vm0.invoke(() -> doOpsOnRegion(regionName, DESTROY));
+ server1.invoke(() -> doOpsOnRegion(regionName, PUT));
+ server1.invoke(() -> doOpsOnRegion(regionName, GET));
+ server1.invoke(() -> doOpsOnRegion(regionName, GET_ENTRY));
+ server1.invoke(() -> doOpsOnRegion(regionName, CONTAINS_KEY));
+ server1.invoke(() -> doOpsOnRegion(regionName, CONTAINS_VALUE_FOR_KEY));
+ server1.invoke(() -> doOpsOnRegion(regionName, INVALIDATE));
+ server1.invoke(() -> doOpsOnRegion(regionName, DESTROY));
- vm0.invoke(() -> validatePartitionedRegionOpsStats(regionName));
+ server1.invoke(() -> validatePartitionedRegionOpsStats(regionName));
- vm0.invoke(() -> validateRedundantCopiesStats(regionName));
+ server1.invoke(() -> validateRedundantCopiesStats(regionName));
}
/**
- * Ok, first problem, GC'd tombstone is counted as an entry To test - modifying a tombstone -
- * modifying and doing tombstone GC?
+ * Test to ensure that tombstone entries are not counted as a part of region entries
*/
@Test
- public void testDataStoreEntryCountWithRebalance() throws Exception {
- vm0.invoke(() -> createPartitionedRegionWithRebalance(regionName, 0));
-
- vm0.invoke(() -> {
- Cache cache = getCache();
+ public void whenEntryIsDestroyedThenTombstoneShouldNotPartOfEntries() {
Review Comment:
Missing word here, should be "whenEntryIsDestroyedThenTombstoneShouldNotBePartOfEntries"
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionStatsDistributedTest.java:
##########
@@ -51,55 +51,67 @@ public class PartitionedRegionStatsDUnitTest extends CacheTestCase {
public static final String STAT_CONFIGURED_REDUNDANT_COPIES = "configuredRedundantCopies";
public static final String STAT_ACTUAL_REDUNDANT_COPIES = "actualRedundantCopies";
- private VM vm0;
- private VM vm1;
- private VM vm2;
- private VM vm3;
-
- private String regionName;
+ @Rule
+ public ClusterStartupRule clusterStartupRule = new ClusterStartupRule();
- @Before
- public void setUp() throws Exception {
- vm0 = getHost(0).getVM(0);
- vm1 = getHost(0).getVM(1);
- vm2 = getHost(0).getVM(2);
- vm3 = getHost(0).getVM(3);
-
- regionName = "region";
- }
+ private final String regionName = "region";
@Test
- public void testClose() throws Exception {
+ public void whenOperationsAreDoneOnEntriesThenStatsMustBeCorrect() {
+ MemberVM locator1 = clusterStartupRule.startLocatorVM(0);
+ int locator1Port = locator1.getPort();
+ MemberVM locator2 =
+ clusterStartupRule.startLocatorVM(1, l -> l.withConnectionToLocator(locator1Port));
+ int locator2Port = locator2.getPort();
Review Comment:
Why are two locators necessary here?
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionStatsDistributedTest.java:
##########
@@ -109,109 +121,129 @@ public void testDataStoreEntryCountWithRebalance() throws Exception {
region.destroy(1);
});
- vm1.invoke(() -> createPartitionedRegionWithRebalance(regionName, 0));
+ server2.invoke(() -> createPartitionedRegionWithRebalance(regionName, 0));
- vm0.invoke(() -> validateEntryCount(regionName, 1));
- vm1.invoke(() -> validateEntryCount(regionName, 1));
+ server1.invoke(() -> validateEntryCount(regionName, 1));
+ server2.invoke(() -> validateEntryCount(regionName, 1));
}
@Test
- public void testDataStoreEntryCount2WithRebalance() throws Exception {
- vm0.invoke(() -> createPartitionedRegionWithRebalance(regionName, 0));
-
- vm0.invoke(() -> {
- Cache cache = getCache();
+ public void whenRegionPutsAreDoneThenStatsShouldBeCorrect() {
Review Comment:
Adding "AfterRebalance" to this test name would better capture what it's doing.
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionStatsDistributedTest.java:
##########
@@ -220,45 +252,48 @@ private void putDataInRegion(final String regionName) {
}
private void createPartitionedRegionWithRedundantCopies(final String regionName) {
- PartitionAttributesFactory paf = new PartitionAttributesFactory();
+ PartitionAttributesFactory<?, ?> paf = new PartitionAttributesFactory<>();
Review Comment:
Rather than `<?, ?>`, could this test parameterize things in a concrete way? Looking at the tests, it seems like the Region always uses `<Integer, Integer>`.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org