[
https://issues.apache.org/jira/browse/GEODE-8119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17181217#comment-17181217
]
ASF GitHub Bot commented on GEODE-8119:
---------------------------------------
jujoramos commented on a change in pull request #5466:
URL: https://github.com/apache/geode/pull/5466#discussion_r474010363
##########
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
##########
@@ -519,6 +520,25 @@ public void
offlineDiskStoreCommandShouldNotCreateFolderIfDiskStoreDoesNotExist(
assertThat(Files.exists(nonExistingDiskStorePath)).isFalse();
}
+ @Test
+ @Parameters({"compact offline-disk-store", "describe offline-disk-store",
+ "upgrade offline-disk-store", "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void offlineDiskStoreCommandShouldNotPassIfDiskStoreFileDoesNotExist(
Review comment:
Can you change the test name to something more meaningful?, maybe
`offlineDiskStoreCommandShouldFailWhenDiskStoreFileDoesNotExist`?.
##########
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/OfflineDiskStoreCommandsDUnitTest.java
##########
@@ -0,0 +1,244 @@
+/*
+ * 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.management.internal.cli.commands;
+
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static
org.apache.geode.distributed.ConfigurationProperties.START_LOCATOR;
+import static
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPortsForDUnitSite;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Serializable;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.DiskInitFile;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+@RunWith(JUnitParamsRunner.class)
+public class OfflineDiskStoreCommandsDUnitTest implements Serializable {
+ private static final String REGION_NAME = "testRegion";
+ private static final String DISK_STORE_ID = "testDisk";
+ private static final String WRONG_DISK_STORE_ID = "wrongTestDisk";
+
+ @Rule
+ public CacheRule cacheRule = new CacheRule();
+
+ @Rule
+ public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+
+ private Properties createLocatorConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+ config.setProperty(START_LOCATOR,
+ "localhost[" + localLocatorPort +
"],server=true,peer=true,hostname-for-clients=localhost");
+
+ return config;
+ }
+
+ private Properties createServerConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+
+ return config;
+ }
+
+ private void createDiskStore(File[] diskStoreDirectories) {
+ DiskStoreFactory diskStoreFactory =
cacheRule.getCache().createDiskStoreFactory();
+ diskStoreFactory.setMaxOplogSize(1);
+ diskStoreFactory.setAutoCompact(true);
+ diskStoreFactory.setAllowForceCompaction(true);
+ diskStoreFactory.setDiskDirs(diskStoreDirectories);
+
+ diskStoreFactory.create(DISK_STORE_ID);
+ }
+
+ private void createRegion() {
+ cacheRule.getCache()
+ .<String,
String>createRegionFactory(RegionShortcut.PARTITION_PERSISTENT)
+ .setDiskStoreName(DISK_STORE_ID)
+ .create(REGION_NAME);
+ }
+
+ private void createServerWithRegionAndPersistentRegion(File[]
diskStoreDirectories) {
+ createDiskStore(diskStoreDirectories);
+ createRegion();
+ cacheRule.getCache().getRegion(REGION_NAME);
+ }
+
+ private void gracefullyDisconnect() {
+
InternalDistributedSystem.getConnectedInstance().stopReconnectingNoDisconnect();
+ InternalDistributedSystem.getConnectedInstance().disconnect();
+ await()
+ .untilAsserted(() ->
assertThat(InternalDistributedSystem.getConnectedInstance()).isNull());
+ }
+
+ @Test
+ @Parameters({"compact offline-disk-store", "describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testOfflineCommandsWithMultipleDirs(String baseCommand) throws
IOException {
Review comment:
Can you change the test name to something more meaningful?, maybe
`offlineDiskStoreCommandsShouldSucceedWhenDiskStoreHasMultipleDirectories`, or
`offlineDiskStoreCommandsSupportDiskStoresWithMultipleDirerctories`?
##########
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/OfflineDiskStoreCommandsDUnitTest.java
##########
@@ -0,0 +1,244 @@
+/*
+ * 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.management.internal.cli.commands;
+
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static
org.apache.geode.distributed.ConfigurationProperties.START_LOCATOR;
+import static
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPortsForDUnitSite;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Serializable;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.DiskInitFile;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+@RunWith(JUnitParamsRunner.class)
+public class OfflineDiskStoreCommandsDUnitTest implements Serializable {
+ private static final String REGION_NAME = "testRegion";
+ private static final String DISK_STORE_ID = "testDisk";
+ private static final String WRONG_DISK_STORE_ID = "wrongTestDisk";
+
+ @Rule
+ public CacheRule cacheRule = new CacheRule();
+
+ @Rule
+ public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+
+ private Properties createLocatorConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+ config.setProperty(START_LOCATOR,
+ "localhost[" + localLocatorPort +
"],server=true,peer=true,hostname-for-clients=localhost");
+
+ return config;
+ }
+
+ private Properties createServerConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+
+ return config;
+ }
+
+ private void createDiskStore(File[] diskStoreDirectories) {
+ DiskStoreFactory diskStoreFactory =
cacheRule.getCache().createDiskStoreFactory();
+ diskStoreFactory.setMaxOplogSize(1);
+ diskStoreFactory.setAutoCompact(true);
+ diskStoreFactory.setAllowForceCompaction(true);
+ diskStoreFactory.setDiskDirs(diskStoreDirectories);
+
+ diskStoreFactory.create(DISK_STORE_ID);
+ }
+
+ private void createRegion() {
+ cacheRule.getCache()
+ .<String,
String>createRegionFactory(RegionShortcut.PARTITION_PERSISTENT)
+ .setDiskStoreName(DISK_STORE_ID)
+ .create(REGION_NAME);
+ }
+
+ private void createServerWithRegionAndPersistentRegion(File[]
diskStoreDirectories) {
+ createDiskStore(diskStoreDirectories);
+ createRegion();
+ cacheRule.getCache().getRegion(REGION_NAME);
+ }
+
+ private void gracefullyDisconnect() {
+
InternalDistributedSystem.getConnectedInstance().stopReconnectingNoDisconnect();
+ InternalDistributedSystem.getConnectedInstance().disconnect();
+ await()
+ .untilAsserted(() ->
assertThat(InternalDistributedSystem.getConnectedInstance()).isNull());
+ }
+
+ @Test
+ @Parameters({"compact offline-disk-store", "describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testOfflineCommandsWithMultipleDirs(String baseCommand) throws
IOException {
+ VM locator = getVM(0);
+ VM server = getVM(1);
+ final int ENTRIES = 100000;
+ int site1Port = getRandomAvailableTCPPortsForDUnitSite(1)[0];
+
+ File diskStoreDirectory1 = temporaryFolder.newFolder("diskDir1");
+ File diskStoreDirectory2 = temporaryFolder.newFolder("diskDir2");
+ File diskStoreDirectory3 = temporaryFolder.newFolder("diskDir3");
+ File[] diskStoreDirectories =
+ new File[] {diskStoreDirectory1, diskStoreDirectory2,
diskStoreDirectory3};
+ String diskDirs =
Arrays.stream(diskStoreDirectories).map(File::getAbsolutePath)
+ .collect(Collectors.joining(","));
+
+ locator.invoke(() ->
cacheRule.createCache(createLocatorConfiguration(site1Port)));
+ server.invoke(() ->
cacheRule.createCache(createServerConfiguration(site1Port)));
+ server.invoke(() -> {
+ createServerWithRegionAndPersistentRegion(diskStoreDirectories);
+ Region<String, String> region =
cacheRule.getCache().getRegion(REGION_NAME);
+ IntStream.range(0, ENTRIES).forEach(i -> region.put("Key_" + i, "Value_"
+ i));
+ });
+ locator.invoke(this::gracefullyDisconnect);
+ server.invoke(this::gracefullyDisconnect);
+ gfsh.executeAndAssertThat(
+ baseCommand + " --name=" + DISK_STORE_ID + " --disk-dirs=" + diskDirs)
+ .statusIsSuccess();
+ }
+
+ @Test
+ @Parameters({"describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testThreadHangWithOfflineDiskStoreCommands(String baseCommand)
throws IOException {
Review comment:
Can you change the test name to something more meaningful?, maybe
`asyncFlusherThreadIsNotStartedForOfflineDiskStoreCommands`, or
`offlineDiskStoreCommandsDoNotLeaveLingeringThreadsRunning`?
##########
File path:
geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java
##########
@@ -55,4 +59,33 @@ public void removeOptionMustBeUsedAlone() {
gfsh.executeAndAssertThat(command, commandString).statusIsError()
.containsOutput("Cannot use the --remove=true parameter with any other
parameters");
}
+
+ @Test
+ public void testDirValidation() throws IOException {
Review comment:
Can you remove `throws IOException` as it's not thrown from within the
method body?.
##########
File path:
geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
##########
@@ -98,4 +106,16 @@ public void commandSucceedsWithNameAndMember() throws
Exception {
gfsh.executeAndAssertThat(cmd).statusIsSuccess()
.containsOutput(expectedData.toArray(new String[0]));
}
+
+ @Test
+ public void testDirValidation() throws Exception {
Review comment:
Can you remove `throws Exception` as it's not thrown from within the
method body?. Since you're already modifying the file, also, it would be great
if you can remove `throws Exception` from every single method in the class as
none actually throws anything 👍 .
##########
File path:
geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
##########
@@ -34,6 +38,7 @@
private static final String REGION_NAME = "test-region";
private static final String MEMBER_NAME = "testServer";
private static final String DISK_STORE_NAME = "testDiskStore";
+ private static final String WRONG_DISK_STORE_NAME = "wrongTestDiskStore";
Review comment:
This variable is not used across the test, can you delete it?.
##########
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/OfflineDiskStoreCommandsDUnitTest.java
##########
@@ -0,0 +1,244 @@
+/*
+ * 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.management.internal.cli.commands;
+
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static
org.apache.geode.distributed.ConfigurationProperties.START_LOCATOR;
+import static
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPortsForDUnitSite;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Serializable;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.DiskInitFile;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+@RunWith(JUnitParamsRunner.class)
+public class OfflineDiskStoreCommandsDUnitTest implements Serializable {
+ private static final String REGION_NAME = "testRegion";
+ private static final String DISK_STORE_ID = "testDisk";
+ private static final String WRONG_DISK_STORE_ID = "wrongTestDisk";
+
+ @Rule
+ public CacheRule cacheRule = new CacheRule();
+
+ @Rule
+ public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+
+ private Properties createLocatorConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+ config.setProperty(START_LOCATOR,
+ "localhost[" + localLocatorPort +
"],server=true,peer=true,hostname-for-clients=localhost");
+
+ return config;
+ }
+
+ private Properties createServerConfiguration(int localLocatorPort) {
+ Properties config = new Properties();
+ config.setProperty(MCAST_PORT, "0");
+ config.setProperty(LOCATORS, "localhost[" + localLocatorPort + ']');
+
+ return config;
+ }
+
+ private void createDiskStore(File[] diskStoreDirectories) {
+ DiskStoreFactory diskStoreFactory =
cacheRule.getCache().createDiskStoreFactory();
+ diskStoreFactory.setMaxOplogSize(1);
+ diskStoreFactory.setAutoCompact(true);
+ diskStoreFactory.setAllowForceCompaction(true);
+ diskStoreFactory.setDiskDirs(diskStoreDirectories);
+
+ diskStoreFactory.create(DISK_STORE_ID);
+ }
+
+ private void createRegion() {
+ cacheRule.getCache()
+ .<String,
String>createRegionFactory(RegionShortcut.PARTITION_PERSISTENT)
+ .setDiskStoreName(DISK_STORE_ID)
+ .create(REGION_NAME);
+ }
+
+ private void createServerWithRegionAndPersistentRegion(File[]
diskStoreDirectories) {
+ createDiskStore(diskStoreDirectories);
+ createRegion();
+ cacheRule.getCache().getRegion(REGION_NAME);
+ }
+
+ private void gracefullyDisconnect() {
+
InternalDistributedSystem.getConnectedInstance().stopReconnectingNoDisconnect();
+ InternalDistributedSystem.getConnectedInstance().disconnect();
+ await()
+ .untilAsserted(() ->
assertThat(InternalDistributedSystem.getConnectedInstance()).isNull());
+ }
+
+ @Test
+ @Parameters({"compact offline-disk-store", "describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testOfflineCommandsWithMultipleDirs(String baseCommand) throws
IOException {
+ VM locator = getVM(0);
+ VM server = getVM(1);
+ final int ENTRIES = 100000;
+ int site1Port = getRandomAvailableTCPPortsForDUnitSite(1)[0];
+
+ File diskStoreDirectory1 = temporaryFolder.newFolder("diskDir1");
+ File diskStoreDirectory2 = temporaryFolder.newFolder("diskDir2");
+ File diskStoreDirectory3 = temporaryFolder.newFolder("diskDir3");
+ File[] diskStoreDirectories =
+ new File[] {diskStoreDirectory1, diskStoreDirectory2,
diskStoreDirectory3};
+ String diskDirs =
Arrays.stream(diskStoreDirectories).map(File::getAbsolutePath)
+ .collect(Collectors.joining(","));
+
+ locator.invoke(() ->
cacheRule.createCache(createLocatorConfiguration(site1Port)));
+ server.invoke(() ->
cacheRule.createCache(createServerConfiguration(site1Port)));
+ server.invoke(() -> {
+ createServerWithRegionAndPersistentRegion(diskStoreDirectories);
+ Region<String, String> region =
cacheRule.getCache().getRegion(REGION_NAME);
+ IntStream.range(0, ENTRIES).forEach(i -> region.put("Key_" + i, "Value_"
+ i));
+ });
+ locator.invoke(this::gracefullyDisconnect);
+ server.invoke(this::gracefullyDisconnect);
+ gfsh.executeAndAssertThat(
+ baseCommand + " --name=" + DISK_STORE_ID + " --disk-dirs=" + diskDirs)
+ .statusIsSuccess();
+ }
+
+ @Test
+ @Parameters({"describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testThreadHangWithOfflineDiskStoreCommands(String baseCommand)
throws IOException {
+ VM locator = getVM(0);
+ VM server = getVM(1);
+ final int ENTRIES = 100000;
+ int site1Port = getRandomAvailableTCPPortsForDUnitSite(1)[0];
+ String threadName = "Asynchronous disk writer for region";
+ int counter = 0;
+
+ File diskStoreDirectory1 = temporaryFolder.newFolder("diskDir1");
+
+ File[] diskStoreDirectories =
+ new File[] {diskStoreDirectory1};
+ String diskDirs =
Arrays.stream(diskStoreDirectories).map(File::getAbsolutePath)
+ .collect(Collectors.joining(","));
+
+ locator.invoke(() ->
cacheRule.createCache(createLocatorConfiguration(site1Port)));
+ server.invoke(() ->
cacheRule.createCache(createServerConfiguration(site1Port)));
+ server.invoke(() -> {
+ createServerWithRegionAndPersistentRegion(diskStoreDirectories);
+ Region<String, String> region =
cacheRule.getCache().getRegion(REGION_NAME);
+ IntStream.range(0, ENTRIES).forEach(i -> region.put("Key_" + i, "Value_"
+ i));
+ });
+ locator.invoke(this::gracefullyDisconnect);
+ server.invoke(this::gracefullyDisconnect);
+
+ gfsh.executeAndAssertThat(
+ baseCommand + " --name=" + DISK_STORE_ID + " --disk-dirs=" + diskDirs)
+ .statusIsSuccess();
+
+ File tempFile = temporaryFolder.newFile("dumpFile.txt");
+ BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile));
+ ThreadMXBean bean = ManagementFactory.getThreadMXBean();
+ ThreadInfo[] infos = bean.dumpAllThreads(true, true);
+ for (ThreadInfo info : infos) {
+ if (info.toString().contains(threadName))
+ writer.append(info.toString());
+ }
+
+ writer.close();
+
+ try (BufferedReader br = new BufferedReader(new FileReader(tempFile))) {
+ String line;
+ while ((line = br.readLine()) != null) {
+ if (line.contains(threadName))
+ counter++;
+ }
+ }
+ assertThat(counter).isEqualTo(0);
+ }
+
+ @Test
+ @Parameters({"compact offline-disk-store", "describe offline-disk-store",
+ "validate offline-disk-store",
+ "alter disk-store --region=testRegion --enable-statistics=true"})
+ public void testOfflineCommandsWithMultipleDirsAndWrongName(String
baseCommand)
Review comment:
Can you change the test name to something more meaningful?, maybe
`offlineDiskStoreCommandShouldFailWhenDiskStoreFileDoesNotExist`?.
##########
File path:
geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java
##########
@@ -55,4 +59,33 @@ public void removeOptionMustBeUsedAlone() {
gfsh.executeAndAssertThat(command, commandString).statusIsError()
.containsOutput("Cannot use the --remove=true parameter with any other
parameters");
}
+
+ @Test
+ public void testDirValidation() throws IOException {
+ CommandStringBuilder csb = new
CommandStringBuilder(CliStrings.ALTER_DISK_STORE);
+ csb.addOption(CliStrings.ALTER_DISK_STORE__DISKSTORENAME, "diskStoreName");
+ csb.addOption(CliStrings.ALTER_DISK_STORE__REGIONNAME, "regionName");
+ csb.addOption(CliStrings.ALTER_DISK_STORE__DISKDIRS, "wrongDiskDir");
+ csb.addOption(CliStrings.ALTER_DISK_STORE__CONCURRENCY__LEVEL, "5");
+ String commandString = csb.toString();
+
+ gfsh.executeAndAssertThat(command, commandString).statusIsError()
+ .containsOutput("Could not find disk-dirs: \"wrongDiskDir");
+ }
+
+ @Test
+ public void testNameValidation() throws IOException {
Review comment:
Can you remove `throws IOException` as it's not thrown from within the
method body?.
----------------------------------------------------------------
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:
us...@infra.apache.org
> Threads are not properly closed when offline disk-store commands are invoked
> ----------------------------------------------------------------------------
>
> Key: GEODE-8119
> URL: https://issues.apache.org/jira/browse/GEODE-8119
> Project: Geode
> Issue Type: Bug
> Components: gfsh
> Reporter: Mario Kevo
> Assignee: Mario Kevo
> Priority: Major
> Labels: pull-request-available
>
> Threads can be opened when you are online and offline, but close only when
> you are online. Once some offline command started thread it cannot be closed
> and after some time if there is a bigger number of this threads it can lead
> to OOM exception.
> Also the problem is that its validating only disk-dirs but not diskStore
> name. So thread can be created but there is no diskStore with that name and
> it will also hang.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
