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/05/31 21:52:07 UTC
[GitHub] [geode] nabarunnag opened a new pull request, #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
nabarunnag opened a new pull request, #7744:
URL: https://github.com/apache/geode/pull/7744
* Renamed to PartitionedRegionStatsDistributedTest
* Used the new test framework
* Compartmentalize the cluster members so that parallel tests do not
affect the individual tests.
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
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
[GitHub] [geode] nabarunnag commented on a diff in pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on code in PR #7744:
URL: https://github.com/apache/geode/pull/7744#discussion_r887227672
##########
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:
Refactored the name
--
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
[GitHub] [geode] nabarunnag commented on pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on PR #7744:
URL: https://github.com/apache/geode/pull/7744#issuecomment-1144427313
Failing test is not related to code change
--
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
[GitHub] [geode] nabarunnag merged pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
nabarunnag merged PR #7744:
URL: https://github.com/apache/geode/pull/7744
--
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
[GitHub] [geode] nabarunnag commented on a diff in pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on code in PR #7744:
URL: https://github.com/apache/geode/pull/7744#discussion_r887220026
##########
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:
Reduced the number of locators
##########
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 added
--
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
[GitHub] [geode] nabarunnag commented on a diff in pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on code in PR #7744:
URL: https://github.com/apache/geode/pull/7744#discussion_r887220667
##########
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:
Made concrete
--
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
[GitHub] [geode] DonalEvans commented on a diff in pull request #7744: GEODE-10347: Refactor PartitionedRegionStatsDUnitTest
Posted by GitBox <gi...@apache.org>.
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