You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jj...@apache.org on 2020/05/25 10:55:19 UTC
[geode] branch develop updated: Revert "GEODE-8119:closing threads
when offline disk store is executed (#5106)" (#5158)
This is an automated email from the ASF dual-hosted git repository.
jjramos pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new c79905f Revert "GEODE-8119:closing threads when offline disk store is executed (#5106)" (#5158)
c79905f is described below
commit c79905f8716d532f57c3ee4a54919a8f28762453
Author: Juan José Ramos <ju...@users.noreply.github.com>
AuthorDate: Mon May 25 11:54:39 2020 +0100
Revert "GEODE-8119:closing threads when offline disk store is executed (#5106)" (#5158)
This reverts commit c3c226a822538cb25e3945b75eef6977a985716e.
---
.../apache/geode/internal/cache/DiskStoreImpl.java | 2 +-
.../cli/commands/DiskStoreCommandsDUnitTest.java | 21 +-----------
.../AlterDiskStoreCommandIntegrationTest.java | 40 ++--------------------
.../DescribeDiskStoreCommandIntegrationTest.java | 36 -------------------
.../cli/commands/AlterOfflineDiskStoreCommand.java | 9 +++--
.../commands/CompactOfflineDiskStoreCommand.java | 9 +++--
.../commands/DescribeOfflineDiskStoreCommand.java | 10 +++---
.../cli/commands/DiskStoreCommandsUtils.java | 18 +++++-----
.../commands/UpgradeOfflineDiskStoreCommand.java | 9 +++--
.../cli/commands/ValidateDiskStoreCommand.java | 9 +++--
10 files changed, 32 insertions(+), 131 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
index 6a137f1..41718d8 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
@@ -2318,7 +2318,7 @@ public class DiskStoreImpl implements DiskStore {
} catch (RuntimeException e) {
rte = e;
}
- if (!isValidating() && !isOfflineCompacting()) {
+ if (!isOffline()) {
try {
// do this before write lock
stopAsyncFlusher();
diff --git a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
index 2623a86..28730d2 100644
--- a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
+++ b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
@@ -66,7 +66,6 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
private static final String GROUP2 = "GROUP2";
private static final String REGION_1 = "REGION1";
private static final String DISKSTORE = "DISKSTORE";
- private static final String IF_FILE_EXT = ".if";
@Rule
public ClusterStartupRule rule = new ClusterStartupRule();
@@ -515,29 +514,11 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
assertThat(Files.exists(nonExistingDiskStorePath)).isFalse();
gfsh.executeAndAssertThat(baseCommand + " --name=" + DISKSTORE + " --disk-dirs="
+ nonExistingDiskStorePath.toAbsolutePath().toString()).statusIsError()
- .containsOutput("Could not find:");
+ .containsOutput("Could not find disk-dirs:");
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(
- String baseCommand) {
- Path diskStorePath =
- Paths.get(tempDir.getRoot().getAbsolutePath());
- assertThat(Files.exists(diskStorePath)).isTrue();
- Path diskStoreFilePath =
- Paths.get(diskStorePath + File.separator + "BACKUPnonExistingDiskStore" + IF_FILE_EXT);
- assertThat(Files.exists(diskStoreFilePath)).isFalse();
- gfsh.executeAndAssertThat(baseCommand + " --name=nonExistingDiskStore --disk-dirs="
- + diskStoreFilePath.toAbsolutePath().toString()).statusIsError()
- .containsOutput("Could not find:");
- assertThat(Files.exists(diskStoreFilePath)).isFalse();
- }
-
- @Test
@Parameters(method = "getDiskDirNames")
public void validateDiskStoreDiskDirectoryPath(String diskDirectoryName)
throws Exception {
diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java
index 677dcf5..30b77cb 100644
--- a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java
+++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java
@@ -17,9 +17,6 @@ package org.apache.geode.management.internal.cli.commands;
import static org.mockito.Mockito.spy;
-import java.io.File;
-import java.io.IOException;
-
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -37,9 +34,8 @@ public class AlterDiskStoreCommandIntegrationTest {
@Rule
public GfshParserRule gfsh = new GfshParserRule();
- private GfshCommand command;
- private static final String IF_FILE_EXT = ".if";
+ private GfshCommand command;
@Before
public void before() {
@@ -47,7 +43,7 @@ public class AlterDiskStoreCommandIntegrationTest {
}
@Test
- public void removeOptionMustBeUsedAlone() throws IOException {
+ public void removeOptionMustBeUsedAlone() {
CommandStringBuilder csb = new CommandStringBuilder(CliStrings.ALTER_DISK_STORE);
csb.addOption(CliStrings.ALTER_DISK_STORE__DISKSTORENAME, "diskStoreName");
csb.addOption(CliStrings.ALTER_DISK_STORE__REGIONNAME, "regionName");
@@ -56,39 +52,7 @@ public class AlterDiskStoreCommandIntegrationTest {
csb.addOption(CliStrings.ALTER_DISK_STORE__REMOVE, "true");
String commandString = csb.toString();
- tempDir.newFile("BACKUPdiskStoreName.if");
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();
-
- File tempFile = tempDir.newFile("BACKUPdiskStoreName" + IF_FILE_EXT);
- gfsh.executeAndAssertThat(command, commandString).statusIsError()
- .containsOutput("Could not find: \"wrongDiskDir" + File.separator + tempFile.getName());
- }
-
- @Test
- public void testNameValidation() throws IOException {
- String diskStoreName = "diskStoreName";
- 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, tempDir.getRoot().toString());
- csb.addOption(CliStrings.ALTER_DISK_STORE__CONCURRENCY__LEVEL, "5");
- String commandString = csb.toString();
-
- gfsh.executeAndAssertThat(command, commandString).statusIsError()
- .containsOutput(
- "Could not find: \"" + tempDir.getRoot().toString() + File.separator + "BACKUP"
- + diskStoreName + IF_FILE_EXT);
- }
-
}
diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
index 1da34ae..c273fcc 100644
--- a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
+++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/DescribeDiskStoreCommandIntegrationTest.java
@@ -15,20 +15,15 @@
package org.apache.geode.management.internal.cli.commands;
-import java.io.File;
import java.util.Arrays;
import java.util.List;
import org.junit.BeforeClass;
import org.junit.ClassRule;
-import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import org.junit.rules.TemporaryFolder;
import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
-import org.apache.geode.management.internal.i18n.CliStrings;
import org.apache.geode.test.junit.categories.PersistenceTest;
import org.apache.geode.test.junit.rules.GfshCommandRule;
import org.apache.geode.test.junit.rules.GfshCommandRule.PortType;
@@ -39,8 +34,6 @@ public class DescribeDiskStoreCommandIntegrationTest {
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";
- private static final String IF_FILE_EXT = ".if";
private static final List<String> expectedData = Arrays.asList("Disk Store ID", "Disk Store Name",
"Member ID", "Member Name", "Allow Force Compaction", "Auto Compaction",
@@ -63,9 +56,6 @@ public class DescribeDiskStoreCommandIntegrationTest {
@ClassRule
public static GfshCommandRule gfsh = new GfshCommandRule().withTimeout(1);
- @Rule
- public TemporaryFolder tempDir = new TemporaryFolder();
-
@Test
public void commandFailsWithoutOptions() throws Exception {
String cmd = "describe disk-store";
@@ -108,30 +98,4 @@ public class DescribeDiskStoreCommandIntegrationTest {
gfsh.executeAndAssertThat(cmd).statusIsSuccess()
.containsOutput(expectedData.toArray(new String[0]));
}
-
- @Test
- public void testDirValidation() throws Exception {
- CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESCRIBE_OFFLINE_DISK_STORE);
- csb.addOption(CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKSTORENAME, DISK_STORE_NAME);
- csb.addOption(CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKDIRS, "wrongDiskDir");
- String commandString = csb.toString();
-
- gfsh.executeAndAssertThat(commandString).statusIsError()
- .containsOutput("Could not find: \"wrongDiskDir" + File.separator + "BACKUP"
- + DISK_STORE_NAME + IF_FILE_EXT);
- }
-
- @Test
- public void testNameValidation() throws Exception {
- CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESCRIBE_OFFLINE_DISK_STORE);
- csb.addOption(CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKSTORENAME, WRONG_DISK_STORE_NAME);
- csb.addOption(CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKDIRS,
- tempDir.getRoot().getAbsolutePath());
- String commandString = csb.toString();
-
- gfsh.executeAndAssertThat(commandString).statusIsError()
- .containsOutput(
- "Could not find: \"" + tempDir.getRoot().toString() + File.separator + "BACKUP"
- + WRONG_DISK_STORE_NAME + IF_FILE_EXT);
- }
}
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java
index 530934a..feaaf89 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java
@@ -60,12 +60,11 @@ public class AlterOfflineDiskStoreCommand extends SingleGfshCommand {
help = CliStrings.ALTER_DISK_STORE__REMOVE__HELP, specifiedDefaultValue = "true",
unspecifiedDefaultValue = "false") boolean remove) {
- String validatedDirectoriesAndFile =
- DiskStoreCommandsUtils.validatedDirectoriesAndFile(diskDirs, diskStoreName);
- if (validatedDirectoriesAndFile != null) {
+ String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs);
+ if (validatedDirectories != null) {
throw new IllegalArgumentException(
- "Could not find: \""
- + validatedDirectoriesAndFile + "\"");
+ "Could not find " + CliStrings.ALTER_DISK_STORE__DISKDIRS + ": \""
+ + validatedDirectories + "\"");
}
try {
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java
index 104d1fc..0cab2c3 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java
@@ -50,12 +50,11 @@ public class CompactOfflineDiskStoreCommand extends SingleGfshCommand {
@CliOption(key = CliStrings.COMPACT_OFFLINE_DISK_STORE__J,
help = CliStrings.COMPACT_OFFLINE_DISK_STORE__J__HELP) String[] jvmProps) {
- String validatedDirectoriesAndFile =
- DiskStoreCommandsUtils.validatedDirectoriesAndFile(diskDirs, diskStoreName);
- if (validatedDirectoriesAndFile != null) {
+ String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs);
+ if (validatedDirectories != null) {
throw new IllegalArgumentException(
- "Could not find: \""
- + validatedDirectoriesAndFile + "\"");
+ "Could not find " + CliStrings.COMPACT_OFFLINE_DISK_STORE__DISKDIRS + ": \""
+ + validatedDirectories + "\"");
}
ResultModel result = new ResultModel();
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java
index 1bc5b8f..f230ece 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java
@@ -43,13 +43,11 @@ public class DescribeOfflineDiskStoreCommand extends SingleGfshCommand {
@CliOption(key = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__REGIONNAME,
help = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__REGIONNAME__HELP) String regionName) {
- String validatedDirectoriesAndFile =
- DiskStoreCommandsUtils.validatedDirectoriesAndFile(diskDirs, diskStoreName);
-
- if (validatedDirectoriesAndFile != null) {
+ String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs);
+ if (validatedDirectories != null) {
throw new IllegalArgumentException(
- "Could not find: \""
- + validatedDirectoriesAndFile + "\"");
+ "Could not find " + CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKDIRS + ": \""
+ + validatedDirectories + "\"");
}
try {
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsUtils.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsUtils.java
index 691d970..720e4e0 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsUtils.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsUtils.java
@@ -34,8 +34,6 @@ import org.apache.geode.management.internal.util.ManagementUtils;
class DiskStoreCommandsUtils {
private static final Logger logger = LogService.getLogger();
- public static final String IF_FILE_EXT = ".if";
-
private static final String LOG4J_CONFIGURATION_FILE_PROPERTY = "log4j.configurationFile";
static void configureLogging(final List<String> commandList) {
@@ -47,24 +45,24 @@ class DiskStoreCommandsUtils {
commandList.add("-D" + LOG4J_CONFIGURATION_FILE_PROPERTY + "=" + configFilePropertyValue);
}
- static String validatedDirectoriesAndFile(String[] diskDirs, String name) {
+ static String validatedDirectories(String[] diskDirs) {
String invalidDirectories = null;
StringBuilder builder = null;
- File diskDirAndFile;
+ File diskDir;
for (String diskDirPath : diskDirs) {
- diskDirAndFile = new File(diskDirPath, "BACKUP" + name + IF_FILE_EXT);
- if (!diskDirAndFile.exists()) {
+ diskDir = new File(diskDirPath);
+ if (!diskDir.exists()) {
if (builder == null) {
builder = new StringBuilder();
} else if (builder.length() != 0) {
builder.append(", ");
}
- builder.append(diskDirAndFile);
- }
- if (builder != null) {
- invalidDirectories = builder.toString();
+ builder.append(diskDirPath);
}
}
+ if (builder != null) {
+ invalidDirectories = builder.toString();
+ }
return invalidDirectories;
}
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java
index 3896233..b3ba2ba 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java
@@ -50,12 +50,11 @@ public class UpgradeOfflineDiskStoreCommand extends SingleGfshCommand {
@CliOption(key = CliStrings.UPGRADE_OFFLINE_DISK_STORE__J,
help = CliStrings.UPGRADE_OFFLINE_DISK_STORE__J__HELP) String[] jvmProps) {
- String validatedDirectoriesAndFileAndFile =
- DiskStoreCommandsUtils.validatedDirectoriesAndFile(diskDirs, diskStoreName);
- if (validatedDirectoriesAndFileAndFile != null) {
+ String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs);
+ if (validatedDirectories != null) {
throw new IllegalArgumentException(
- "Could not find: \""
- + validatedDirectoriesAndFileAndFile + "\"");
+ "Could not find " + CliStrings.UPGRADE_OFFLINE_DISK_STORE__DISKDIRS + ": \""
+ + validatedDirectories + "\"");
}
ResultModel result = new ResultModel();
diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java
index 98f1578..9df1e12 100644
--- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java
+++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java
@@ -46,12 +46,11 @@ public class ValidateDiskStoreCommand extends GfshCommand {
@CliOption(key = CliStrings.VALIDATE_DISK_STORE__J,
help = CliStrings.VALIDATE_DISK_STORE__J__HELP) String[] jvmProps) {
- String validatedDirectoriesAndFile =
- DiskStoreCommandsUtils.validatedDirectoriesAndFile(diskDirs, diskStoreName);
- if (validatedDirectoriesAndFile != null) {
+ String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs);
+ if (validatedDirectories != null) {
throw new IllegalArgumentException(
- "Could not find: \""
- + validatedDirectoriesAndFile + "\"");
+ "Could not find " + CliStrings.VALIDATE_DISK_STORE__DISKDIRS + ": \""
+ + validatedDirectories + "\"");
}
ResultModel result = new ResultModel();