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 2020/05/30 00:21:25 UTC
[GitHub] [geode] jchen21 opened a new pull request #5189: GEODE-7669 Test coverage for Partitioned Region clear with Overflow enabled
jchen21 opened a new pull request #5189:
URL: https://github.com/apache/geode/pull/5189
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jchen21 merged pull request #5189: GEODE-7669 Test coverage for Partitioned Region clear with Overflow enabled
Posted by GitBox <gi...@apache.org>.
jchen21 merged pull request #5189:
URL: https://github.com/apache/geode/pull/5189
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jchen21 commented on a change in pull request #5189: GEODE-7669 Test coverage for Partitioned Region clear with Overflow enabled
Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5189:
URL: https://github.com/apache/geode/pull/5189#discussion_r434906128
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionOverflowClearDUnitTest.java
##########
@@ -0,0 +1,356 @@
+/*
+ * 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.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT;
+import static org.apache.geode.distributed.ConfigurationProperties.USE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.apache.geode.test.dunit.VM.getVMId;
+import static org.apache.geode.test.dunit.VM.toArray;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.Serializable;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.IntStream;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.distributed.LocatorLauncher;
+import org.apache.geode.distributed.ServerLauncher;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.AsyncInvocation;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
+
+public class PartitionedRegionOverflowClearDUnitTest implements Serializable {
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public SerializableTemporaryFolder temporaryFolder = new SerializableTemporaryFolder();
+
+ @Rule
+ public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+ private VM server1;
+ private VM server2;
+ private VM accessor;
+ private VM client;
+
+ private static final String LOCATOR_NAME = "locator";
+ private static final String SERVER1_NAME = "server1";
+ private static final String SERVER2_NAME = "server2";
+ private static final String SERVER3_NAME = "server3";
+
+ private File locatorDir;
+ private File server1Dir;
+ private File server2Dir;
+ private File server3Dir;
+
+ private String locatorString;
+
+ private int locatorPort;
+ private int locatorJmxPort;
+ private int locatorHttpPort;
+ private int serverPort1;
+ private int serverPort2;
+ private int serverPort3;
+
+ private static final AtomicReference<LocatorLauncher> LOCATOR_LAUNCHER = new AtomicReference<>();
+
+ private static final AtomicReference<ServerLauncher> SERVER_LAUNCHER = new AtomicReference<>();
+
+ private static final String OVERFLOW_REGION_NAME = "testOverflowRegion";
+
+ public static final int NUM_ENTRIES = 1000;
+
+ @Before
+ public void setup() throws Exception {
+ VM locator = getVM(0);
+ server1 = getVM(1);
+ server2 = getVM(2);
+ accessor = getVM(3);
+ client = getVM(4);
+
+ locatorDir = temporaryFolder.newFolder(LOCATOR_NAME);
+ server1Dir = temporaryFolder.newFolder(SERVER1_NAME);
+ server2Dir = temporaryFolder.newFolder(SERVER2_NAME);
+ server3Dir = temporaryFolder.newFolder(SERVER3_NAME);
+
+ int[] ports = getRandomAvailableTCPPorts(6);
+ locatorPort = ports[0];
+ locatorJmxPort = ports[1];
+ locatorHttpPort = ports[2];
+ serverPort1 = ports[3];
+ serverPort2 = ports[4];
+ serverPort3 = ports[5];
+
+ locator.invoke(
+ () -> startLocator(locatorDir, locatorPort, locatorJmxPort, locatorHttpPort));
+ gfsh.connectAndVerify(locatorJmxPort, GfshCommandRule.PortType.jmxManager);
+
+ locatorString = "localhost[" + locatorPort + "]";
+ server1.invoke(() -> startServer(SERVER1_NAME, server1Dir, locatorString, serverPort1));
+ server2.invoke(() -> startServer(SERVER2_NAME, server2Dir, locatorString, serverPort2));
+ }
+
+ @Test
+ public void testGfshClearRegionWithOverflow() throws InterruptedException {
+ createPartitionRedundantPersistentOverflowRegion();
+
+ populateRegion();
+ assertRegionSize(NUM_ENTRIES);
+
+ gfsh.executeAndAssertThat("clear region --name=" + OVERFLOW_REGION_NAME).statusIsSuccess();
+ assertRegionSize(0);
+
+ restartServers();
+
+ assertRegionSize(0);
+
+ destroyRegion();
Review comment:
Changes have been made per your suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #5189: GEODE-7669 Test coverage for Partitioned Region clear with Overflow enabled
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5189:
URL: https://github.com/apache/geode/pull/5189#discussion_r434767661
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionOverflowClearDUnitTest.java
##########
@@ -0,0 +1,356 @@
+/*
+ * 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.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT;
+import static org.apache.geode.distributed.ConfigurationProperties.USE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.apache.geode.test.dunit.VM.getVMId;
+import static org.apache.geode.test.dunit.VM.toArray;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.Serializable;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.IntStream;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.distributed.LocatorLauncher;
+import org.apache.geode.distributed.ServerLauncher;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.AsyncInvocation;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
+
+public class PartitionedRegionOverflowClearDUnitTest implements Serializable {
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public SerializableTemporaryFolder temporaryFolder = new SerializableTemporaryFolder();
+
+ @Rule
+ public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+ private VM server1;
+ private VM server2;
+ private VM accessor;
+ private VM client;
+
+ private static final String LOCATOR_NAME = "locator";
+ private static final String SERVER1_NAME = "server1";
+ private static final String SERVER2_NAME = "server2";
+ private static final String SERVER3_NAME = "server3";
+
+ private File locatorDir;
+ private File server1Dir;
+ private File server2Dir;
+ private File server3Dir;
+
+ private String locatorString;
+
+ private int locatorPort;
+ private int locatorJmxPort;
+ private int locatorHttpPort;
+ private int serverPort1;
+ private int serverPort2;
+ private int serverPort3;
+
+ private static final AtomicReference<LocatorLauncher> LOCATOR_LAUNCHER = new AtomicReference<>();
+
+ private static final AtomicReference<ServerLauncher> SERVER_LAUNCHER = new AtomicReference<>();
+
+ private static final String OVERFLOW_REGION_NAME = "testOverflowRegion";
+
+ public static final int NUM_ENTRIES = 1000;
+
+ @Before
+ public void setup() throws Exception {
+ VM locator = getVM(0);
+ server1 = getVM(1);
+ server2 = getVM(2);
+ accessor = getVM(3);
+ client = getVM(4);
+
+ locatorDir = temporaryFolder.newFolder(LOCATOR_NAME);
+ server1Dir = temporaryFolder.newFolder(SERVER1_NAME);
+ server2Dir = temporaryFolder.newFolder(SERVER2_NAME);
+ server3Dir = temporaryFolder.newFolder(SERVER3_NAME);
+
+ int[] ports = getRandomAvailableTCPPorts(6);
+ locatorPort = ports[0];
+ locatorJmxPort = ports[1];
+ locatorHttpPort = ports[2];
+ serverPort1 = ports[3];
+ serverPort2 = ports[4];
+ serverPort3 = ports[5];
+
+ locator.invoke(
+ () -> startLocator(locatorDir, locatorPort, locatorJmxPort, locatorHttpPort));
+ gfsh.connectAndVerify(locatorJmxPort, GfshCommandRule.PortType.jmxManager);
+
+ locatorString = "localhost[" + locatorPort + "]";
+ server1.invoke(() -> startServer(SERVER1_NAME, server1Dir, locatorString, serverPort1));
+ server2.invoke(() -> startServer(SERVER2_NAME, server2Dir, locatorString, serverPort2));
+ }
+
+ @Test
+ public void testGfshClearRegionWithOverflow() throws InterruptedException {
+ createPartitionRedundantPersistentOverflowRegion();
+
+ populateRegion();
+ assertRegionSize(NUM_ENTRIES);
+
+ gfsh.executeAndAssertThat("clear region --name=" + OVERFLOW_REGION_NAME).statusIsSuccess();
+ assertRegionSize(0);
+
+ restartServers();
+
+ assertRegionSize(0);
+
+ destroyRegion();
Review comment:
I think you need a tearDown method to ensure that Locator and Servers are all stopped in every VM even if any tests failed. You could probably move destroyRegion and destroyDiskStore to tearDown as well (or remove them?).
Close clients 1st, then servers, then locator (see the order of VMs in Arrays.asList):
```
@After
public void tearDown() throws Exception {
for (VM vm : Arrays.asList(getVM(4), getVM(3), getVM(2), getVM(1), getVM(0)) {
vm.invoke(() -> {
if (clientCache != null) {
clientCache.close();
}
if (LOCATOR_LAUNCHER.get() != null) {
LOCATOR_LAUNCHER.get().stop();
}
if (SERVER_LAUNCHER.get() != null) {
SERVER_LAUNCHER.get().stop();
}
clientCache = null;
LOCATOR_LAUNCHER.set(null);
SERVER_LAUNCHER.set(null);
}
}
}
```
You'll need to hoist anything (such as clientCache) to be a `private static` field/var in the test class so that you can reference it from tearDown() to close it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org