[
https://issues.apache.org/jira/browse/GEODE-2567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16259963#comment-16259963
]
ASF GitHub Bot commented on GEODE-2567:
---------------------------------------
jdeppe-pivotal closed pull request #1080: GEODE-2567: Add --if-exists to
destroy disk-store
URL: https://github.com/apache/geode/pull/1080
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
index fdf796f226..42d1ecde70 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
@@ -22,7 +22,6 @@
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
-import org.apache.geode.SystemFailure;
import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.management.cli.CliMetaData;
@@ -31,6 +30,7 @@
import org.apache.geode.management.internal.cli.CliUtil;
import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import
org.apache.geode.management.internal.cli.functions.DestroyDiskStoreFunction;
+import
org.apache.geode.management.internal.cli.functions.DestroyDiskStoreFunctionArgs;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.ResultBuilder;
import org.apache.geode.management.internal.cli.result.TabularResultData;
@@ -48,50 +48,44 @@ public Result destroyDiskStore(
help = CliStrings.DESTROY_DISK_STORE__NAME__HELP) String name,
@CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
help = CliStrings.DESTROY_DISK_STORE__GROUP__HELP,
- optionContext = ConverterHint.MEMBERGROUP) String[] groups) {
- try {
- TabularResultData tabularData = ResultBuilder.createTabularResultData();
- boolean accumulatedData = false;
+ optionContext = ConverterHint.MEMBERGROUP) String[] groups,
+ @CliOption(key = CliStrings.IFEXISTS, help = CliStrings.IFEXISTS_HELP,
+ specifiedDefaultValue = "true", unspecifiedDefaultValue = "false")
boolean ifExist) {
- Set<DistributedMember> targetMembers = CliUtil.findMembers(groups, null);
+ TabularResultData tabularData = ResultBuilder.createTabularResultData();
- if (targetMembers.isEmpty()) {
- return
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
- }
+ Set<DistributedMember> targetMembers = CliUtil.findMembers(groups, null);
- ResultCollector<?, ?> rc = CliUtil.executeFunction(new
DestroyDiskStoreFunction(),
- new Object[] {name}, targetMembers);
- List<CliFunctionResult> results =
CliFunctionResult.cleanResults((List<?>) rc.getResult());
+ if (targetMembers.isEmpty()) {
+ return
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+ }
- AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
- for (CliFunctionResult result : results) {
- if (result.isSuccessful()) {
- if (xmlEntity.get() == null) {
- xmlEntity.set(result.getXmlEntity());
- }
- } else {
- tabularData.setStatus(Result.Status.ERROR);
- }
- tabularData.accumulate("Member", result.getMemberIdOrName());
- tabularData.accumulate("Result", result.getMessage());
- }
+ DestroyDiskStoreFunctionArgs functionArgs = new
DestroyDiskStoreFunctionArgs(name, ifExist);
- Result result = ResultBuilder.buildResult(tabularData);
+ ResultCollector<?, ?> rc =
+ CliUtil.executeFunction(new DestroyDiskStoreFunction(), functionArgs,
targetMembers);
+ List<CliFunctionResult> results = CliFunctionResult.cleanResults((List<?>)
rc.getResult());
- if (xmlEntity.get() != null) {
- persistClusterConfiguration(result,
- () -> getSharedConfiguration().deleteXmlEntity(xmlEntity.get(),
groups));
+ AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+ for (CliFunctionResult result : results) {
+ if (result.isSuccessful()) {
+ if (xmlEntity.get() == null) {
+ xmlEntity.set(result.getXmlEntity());
+ }
+ } else {
+ tabularData.setStatus(Result.Status.ERROR);
}
+ tabularData.accumulate("Member", result.getMemberIdOrName());
+ tabularData.accumulate("Result", result.getMessage());
+ }
- return result;
- } catch (VirtualMachineError e) {
- SystemFailure.initiateFailure(e);
- throw e;
- } catch (Throwable th) {
- SystemFailure.checkFailure();
- return ResultBuilder.createGemFireErrorResult(
-
CliStrings.format(CliStrings.DESTROY_DISK_STORE__ERROR_WHILE_DESTROYING_REASON_0,
- new Object[] {th.getMessage()}));
+ Result result = ResultBuilder.buildResult(tabularData);
+
+ if (xmlEntity.get() != null) {
+ persistClusterConfiguration(result,
+ () -> getSharedConfiguration().deleteXmlEntity(xmlEntity.get(),
groups));
}
+
+ return result;
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunction.java
index 741a417a00..6f153017a7 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunction.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunction.java
@@ -14,10 +14,6 @@
*/
package org.apache.geode.management.internal.cli.functions;
-import org.apache.logging.log4j.Logger;
-
-import org.apache.geode.SystemFailure;
-import org.apache.geode.cache.CacheClosedException;
import org.apache.geode.cache.DiskStore;
import org.apache.geode.cache.execute.Function;
import org.apache.geode.cache.execute.FunctionContext;
@@ -25,7 +21,6 @@
import org.apache.geode.internal.InternalEntity;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.xmlcache.CacheXml;
-import org.apache.geode.internal.logging.LogService;
import org.apache.geode.management.internal.configuration.domain.XmlEntity;
/**
@@ -33,64 +28,46 @@
*
* @since GemFire 8.0
*/
-public class DestroyDiskStoreFunction implements Function<Object[]>,
InternalEntity {
- private static final Logger logger = LogService.getLogger();
-
+public class DestroyDiskStoreFunction implements Function, InternalEntity {
private static final long serialVersionUID = 1L;
@Override
- public void execute(FunctionContext<Object[]> context) {
+ public void execute(FunctionContext context) {
// Declared here so that it's available when returning a Throwable
- String memberId = "";
+ String memberId;
- try {
- final Object[] args = context.getArguments();
- final String diskStoreName = (String) args[0];
+ final DestroyDiskStoreFunctionArgs args = (DestroyDiskStoreFunctionArgs)
context.getArguments();
- InternalCache cache = (InternalCache) context.getCache();
+ InternalCache cache = (InternalCache) context.getCache();
- DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
+ DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
- memberId = member.getId();
- // If they set a name use it instead
- if (!member.getName().equals("")) {
- memberId = member.getName();
- }
+ memberId = member.getId();
+ // If they set a name use it instead
+ if (!member.getName().equals("")) {
+ memberId = member.getName();
+ }
- DiskStore diskStore = cache.findDiskStore(diskStoreName);
+ DiskStore diskStore = cache.findDiskStore(args.getId());
- CliFunctionResult result;
+ CliFunctionResult result;
+ try {
if (diskStore != null) {
- XmlEntity xmlEntity = new XmlEntity(CacheXml.DISK_STORE, "name",
diskStoreName);
+ XmlEntity xmlEntity = new XmlEntity(CacheXml.DISK_STORE, "name",
args.getId());
diskStore.destroy();
result = new CliFunctionResult(memberId, xmlEntity, "Success");
} else {
- result = new CliFunctionResult(memberId, false, "Disk store not found
on this member");
+ if (args.isIfExists()) {
+ result = new CliFunctionResult(memberId, true,
+ "Skipping: Disk store not found on this member");
+ } else {
+ result = new CliFunctionResult(memberId, false, "Disk store not
found on this member");
+ }
}
- context.getResultSender().lastResult(result);
-
- } catch (IllegalStateException isex) {
- CliFunctionResult result = new CliFunctionResult(memberId, false,
isex.getMessage());
- context.getResultSender().lastResult(result);
-
- } catch (CacheClosedException cce) {
- CliFunctionResult result = new CliFunctionResult(memberId, false, null);
- context.getResultSender().lastResult(result);
-
- } catch (VirtualMachineError e) {
- SystemFailure.initiateFailure(e);
- throw e;
- } catch (Throwable th) {
- SystemFailure.checkFailure();
- logger.error("Could not destroy disk store: {}", th.getMessage(), th);
-
- CliFunctionResult result = new CliFunctionResult(memberId, th, null);
- context.getResultSender().lastResult(result);
+ } catch (IllegalStateException ex) {
+ result = new CliFunctionResult(memberId, false, ex.getMessage());
}
+ context.getResultSender().lastResult(result);
}
- @Override
- public String getId() {
- return DestroyDiskStoreFunction.class.getName();
- }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunctionArgs.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunctionArgs.java
new file mode 100644
index 0000000000..e338167326
--- /dev/null
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyDiskStoreFunctionArgs.java
@@ -0,0 +1,38 @@
+/*
+ * 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.functions;
+
+import java.io.Serializable;
+
+public class DestroyDiskStoreFunctionArgs implements Serializable {
+
+ private static final long serialVersionUID = 2296397958405313306L;
+ private String id;
+ private boolean ifExists;
+
+
+ public DestroyDiskStoreFunctionArgs(String id, boolean ifExists) {
+ this.id = id;
+ this.ifExists = ifExists;
+ }
+
+ public String getId() {
+ return id;
+ }
+
+ public boolean isIfExists() {
+ return ifExists;
+ }
+}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
index 448579f3b0..2acddf59fb 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
@@ -291,4 +291,20 @@ public void testBackupDiskStore() throws Exception {
.tableHasColumnWithExactValuesInAnyOrder("Member", "locator-0",
"server-1");
}
+ @Test
+ public void destroyDiskStoreIsIdempotent() throws Exception {
+ MemberVM locator = rule.startLocatorVM(0);
+ MemberVM server1 = rule.startServerVM(1, locator.getPort());
+
+ gfsh.connectAndVerify(locator);
+
+ gfsh.executeAndAssertThat(
+ String.format("create disk-store --name=%s --dir=%s", DISKSTORE,
DISKSTORE))
+ .statusIsSuccess();
+
+ gfsh.executeAndAssertThat(String.format("destroy disk-store --name=%s
--if-exists", DISKSTORE))
+ .statusIsSuccess();
+ gfsh.executeAndAssertThat(String.format("destroy disk-store --name=%s
--if-exists", DISKSTORE))
+ .statusIsSuccess();
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
> gfsh destroy disk-store should be idempotent
> --------------------------------------------
>
> Key: GEODE-2567
> URL: https://issues.apache.org/jira/browse/GEODE-2567
> Project: Geode
> Issue Type: Sub-task
> Components: gfsh
> Reporter: Swapnil Bawaskar
>
> Currently, gfsh {{destroy disk-store}} does *not* throw an error when run
> multiple times:
> {{{
> gfsh>destroy disk-store --name=foo
> Member | Result
> ------ | -----------------------------------
> serv1 | Disk store not found on this member
> serv2 | Disk store not found on this member
> }}}
> However, for a uniform user experience, it should. We should then add a
> {{--if-exists}} option to the command to make it idempotent.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
