You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2019/05/15 15:52:50 UTC
[geode] branch develop updated: GEODE-6749: Prevent gfsh from
creating duplicate named disk stores (#3562)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe 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 690c65e GEODE-6749: Prevent gfsh from creating duplicate named disk stores (#3562)
690c65e is described below
commit 690c65e86f9f33ed12b34a7e6ae831fcc39a58c6
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Wed May 15 08:52:36 2019 -0700
GEODE-6749: Prevent gfsh from creating duplicate named disk stores (#3562)
---
.../cli/commands/DiskStoreCommandsDUnitTest.java | 33 +++++++++++--
.../cli/commands/CreateDiskStoreCommand.java | 56 ++++++++++++++++++++++
.../cli/commands/CreateDiskStoreCommandTest.java | 5 ++
3 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
index ec14a15..4abb2e8 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
@@ -76,9 +76,17 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
public transient TemporaryFolder tempDir = new TemporaryFolder();
private void createDiskStoreAndRegion(MemberVM jmxManager, int serverCount) {
+ createDiskStore(jmxManager, serverCount);
+
+ gfsh.executeAndAssertThat(String.format(
+ "create region --name=%s --type=REPLICATE_PERSISTENT --disk-store=%s --group=%s --eviction-action=overflow-to-disk",
+ REGION_1, DISKSTORE, GROUP)).statusIsSuccess();
+ }
+
+ private void createDiskStore(MemberVM jmxManager, int serverCount) {
gfsh.executeAndAssertThat(String.format(
"create disk-store --name=%s --dir=%s --group=%s --auto-compact=false --compaction-threshold=99 --max-oplog-size=1 --allow-force-compaction=true",
- DISKSTORE, DISKSTORE, GROUP));
+ DISKSTORE, DISKSTORE, GROUP)).statusIsSuccess();
List<String> diskStores =
IntStream.rangeClosed(1, serverCount).mapToObj(x -> DISKSTORE).collect(Collectors.toList());
@@ -86,10 +94,6 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
.tableHasColumnWithValuesContaining("Disk Store Name", diskStores.toArray(new String[0]));
jmxManager.waitUntilDiskStoreIsReadyOnExactlyThisManyServers(DISKSTORE, serverCount);
-
- gfsh.executeAndAssertThat(String.format(
- "create region --name=%s --type=REPLICATE_PERSISTENT --disk-store=%s --group=%s --eviction-action=overflow-to-disk",
- REGION_1, DISKSTORE, GROUP)).statusIsSuccess();
}
private static SerializableRunnableIF dataProducer() {
@@ -102,6 +106,25 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
}
@Test
+ public void createDuplicateDiskStoreFails() throws Exception {
+ Properties props = new Properties();
+ props.setProperty("groups", GROUP);
+
+ MemberVM locator = rule.startLocatorVM(0);
+ MemberVM server1 = rule.startServerVM(1, props, locator.getPort());
+
+ gfsh.connectAndVerify(locator);
+
+ createDiskStore(locator, 1);
+
+ gfsh.executeAndAssertThat(String.format(
+ "create disk-store --name=%s --dir=%s --group=%s --auto-compact=false --compaction-threshold=99 --max-oplog-size=1 --allow-force-compaction=true",
+ DISKSTORE, DISKSTORE, GROUP))
+ .statusIsError()
+ .containsOutput("Error: Disk store DISKSTORE already exists");
+ }
+
+ @Test
public void testMissingDiskStore() throws Exception {
Properties props = new Properties();
props.setProperty("groups", GROUP);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
index 18e57ea..329d149 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
@@ -17,22 +17,30 @@ package org.apache.geode.management.internal.cli.commands;
import java.io.File;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
+import org.apache.commons.lang3.tuple.Pair;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
+import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.DiskDirType;
import org.apache.geode.cache.configuration.DiskStoreType;
+import org.apache.geode.cache.execute.Execution;
+import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.internal.cache.DiskStoreAttributes;
+import org.apache.geode.internal.cache.execute.AbstractExecution;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
import org.apache.geode.management.cli.SingleGfshCommand;
+import org.apache.geode.management.internal.cli.domain.DiskStoreDetails;
import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.functions.CreateDiskStoreFunction;
+import org.apache.geode.management.internal.cli.functions.ListDiskStoresFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.management.internal.security.ResourceOperation;
@@ -113,6 +121,12 @@ public class CreateDiskStoreCommand extends SingleGfshCommand {
return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
}
+ Pair<Boolean, String> validationResult =
+ validateDiskstoreAttributes(diskStoreAttributes, targetMembers);
+ if (validationResult.getLeft().equals(Boolean.FALSE)) {
+ return ResultModel.createError(validationResult.getRight());
+ }
+
List<CliFunctionResult> functionResults = executeAndGetFunctionResult(
new CreateDiskStoreFunction(), new Object[] {name, diskStoreAttributes}, targetMembers);
@@ -122,6 +136,22 @@ public class CreateDiskStoreCommand extends SingleGfshCommand {
return result;
}
+ @VisibleForTesting
+ Pair<Boolean, String> validateDiskstoreAttributes(
+ DiskStoreAttributes diskStoreAttributes,
+ Set<DistributedMember> targetMembers) {
+ List<DiskStoreDetails> currentDiskstores = getDiskStoreListing(targetMembers);
+
+ for (DiskStoreDetails detail : currentDiskstores) {
+ if (detail.getName().equals(diskStoreAttributes.getName())) {
+ return Pair.of(Boolean.FALSE,
+ String.format("Error: Disk store %s already exists", diskStoreAttributes.getName()));
+ }
+ }
+
+ return Pair.of(Boolean.TRUE, null);
+ }
+
private DiskStoreType createDiskStoreType(String name, DiskStoreAttributes diskStoreAttributes) {
DiskStoreType diskStoreType = new DiskStoreType();
diskStoreType.setAllowForceCompaction(diskStoreAttributes.getAllowForceCompaction());
@@ -151,6 +181,32 @@ public class CreateDiskStoreCommand extends SingleGfshCommand {
return diskStoreType;
}
+ @SuppressWarnings("unchecked")
+ List<DiskStoreDetails> getDiskStoreListing(Set<DistributedMember> members) {
+ final Execution membersFunctionExecutor = getMembersFunctionExecutor(members);
+ if (membersFunctionExecutor instanceof AbstractExecution) {
+ ((AbstractExecution) membersFunctionExecutor).setIgnoreDepartedMembers(true);
+ }
+
+ final ResultCollector<?, ?> resultCollector =
+ membersFunctionExecutor.execute(new ListDiskStoresFunction());
+
+ final List<?> results = (List<?>) resultCollector.getResult();
+ final List<DiskStoreDetails> distributedSystemMemberDiskStores =
+ new ArrayList<>(results.size());
+
+ for (final Object result : results) {
+ if (result instanceof Set) {
+ distributedSystemMemberDiskStores.addAll((Set<DiskStoreDetails>) result);
+ }
+ }
+
+ Collections.sort(distributedSystemMemberDiskStores);
+
+ return distributedSystemMemberDiskStores;
+ }
+
+
@Override
public boolean updateConfigForGroup(String group, CacheConfig config, Object configObject) {
DiskStoreType diskStoreType = (DiskStoreType) configObject;
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
index d8042c9..a65340c 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
@@ -23,6 +23,7 @@ import static org.mockito.Mockito.spy;
import java.util.Collections;
+import org.apache.commons.lang3.tuple.Pair;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
@@ -52,6 +53,8 @@ public class CreateDiskStoreCommandTest {
any());
doReturn(Collections.singletonList(mock(CliFunctionResult.class))).when(command)
.executeAndGetFunctionResult(any(), any(), any());
+ doReturn(Pair.of(Boolean.TRUE, null)).when(command).validateDiskstoreAttributes(any(),
+ any());
ResultModel resultModel =
gfsh.executeAndAssertThat(command, "create disk-store --name=ds1 --dir=./data/persist")
.getResultModel();
@@ -67,6 +70,8 @@ public class CreateDiskStoreCommandTest {
any());
doReturn(Collections.singletonList(mock(CliFunctionResult.class))).when(command)
.executeAndGetFunctionResult(any(), any(), any());
+ doReturn(Pair.of(Boolean.TRUE, null)).when(command).validateDiskstoreAttributes(any(),
+ any());
ResultModel resultModel =
gfsh.executeAndAssertThat(command, "create disk-store --name=ds1 --dir=/data/persist")
.getResultModel();