zstan commented on code in PR #13577:
URL: https://github.com/apache/ignite/pull/13577#discussion_r4081591189


##########
docs/_docs/snapshots/snapshots.adoc:
##########
@@ -287,6 +287,49 @@ control.(sh|bat) --snapshot restore snapshot_09062021 
--groups cache-group1,cach
 control.(sh|bat) --snapshot restore snapshot_09062021 --increment 1
 ----
 
+== Deleting Snapshot
+
+You can delete a snapshot using the `control.sh|bat` script.
+
+The deletion is performed on all *online* server nodes of the cluster.
+[NOTE]
+====
+The snapshot integrity, topology and correctness aren't checked. Snapshot data 
on offline server nodes aren't deleted.
+====
+
+[tabs]
+--
+tab:Unix[]
+[source,shell]
+----
+# Delete the snapshot "snapshot_09062021".
+control.sh --snapshot delete snapshot_09062021
+
+# Delete the snapshot "snapshot_09062021" located in the 
"/tmp/ignite/snapshots" folder.
+control.sh --snapshot delete snapshot_09062021 --src /tmp/ignite/snapshots
+----
+
+tab:Windows[]
+[source,shell]
+----
+# Delete the snapshot "snapshot_09062021".
+control.bat --snapshot delete snapshot_09062021
+
+# Delete the snapshot "snapshot_09062021" located in the 
"/tmp/ignite/snapshots" folder.
+control.bat --snapshot delete snapshot_09062021 --src /tmp/ignite/snapshots
+----
+--
+
+=== Delete operation limitations
+
+The delete operation is subject to the following limitations:
+
+* The deletion is rejected if any snapshot operation with the same name 
(create, restore, check, or a delete with the same name) is
+  active for the snapshot.
+* The operation cannot be undone and the deleted snapshot cannot be restored. 
The command prompts for a confirmation.
+* Before deletion, no snapshot validation is done except finding and reading 
its metadata. If the metadata isn't found or cannot be read, snapshot isn't 
deleted.
+* If the provided snapshot path belongs to Ignite's home or to Ignite's a not 
snapshot work directory, the operation fails.

Review Comment:
   why do we need this additional check ? You think someone can put snap meta 
file into Ignite home dir ?



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";

Review Comment:
   ```suggestion
       public static final String DESC = "Deletes the snapshot and all its 
incremental snapshots from all online server nodes";
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";
+
+    /** */
+    public static final String UNSURED_DELETION_PREF = "WARNING: the following 
nodes found snapshot data but might not " +
+        "remove it completely ";
+
+    /** */
+    public static final String REMOVED_PREF = "Snapshot removal is completed 
on ";
+
+    /** */
+    public static final String NODE_NOT_FOUND_PREF = "NOTE: the following 
nodes can't find any snapshot data, " +
+        "operation skipped ";
+
+    /** */
+    public static final String NOT_FOUND_PREF = "Snapshot not found on current 
server nodes ";

Review Comment:
   ```suggestion
       public static final String NOT_FOUND_PREF = "Snapshot not found on 
available server nodes";
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";
+
+    /** */
+    public static final String UNSURED_DELETION_PREF = "WARNING: the following 
nodes found snapshot data but might not " +
+        "remove it completely ";
+
+    /** */
+    public static final String REMOVED_PREF = "Snapshot removal is completed 
on ";
+
+    /** */
+    public static final String NODE_NOT_FOUND_PREF = "NOTE: the following 
nodes can't find any snapshot data, " +
+        "operation skipped ";
+
+    /** */
+    public static final String NOT_FOUND_PREF = "Snapshot not found on current 
server nodes ";
+
+    /** */
+    public static final String MISSING_BASELINES = "WARNING: the snapshot's 
baseline nodes with the following consistent " +
+        "ids are missing in current cluster ";
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public String description() {
+        return DESC;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteCommandArg> argClass() {
+        return SnapshotDeleteCommandArg.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteTask> taskClass() {
+        return SnapshotDeleteTask.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public void printResult(SnapshotDeleteCommandArg arg, 
SnapshotDeleteProcessResult res, Consumer<String> printer) {
+        boolean found = false;
+
+        if (!res.uncompletedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(UNSURED_DELETION_PREF + 
nodeIdPairsStrLst(res.uncompletedNodes()));
+
+            printer.accept("");
+        }
+
+        if (!res.completedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(REMOVED_PREF + 
nodeIdPairsStrLst(res.completedNodes()));
+            printer.accept("");
+        }
+
+        if (found) {
+            if (!res.emptyNodes().isEmpty())
+                printer.accept(NODE_NOT_FOUND_PREF + 
nodeIdPairsStrLst(res.emptyNodes()));
+
+            if (!res.absentBaselines().isEmpty())
+                printer.accept(MISSING_BASELINES + 
nodeIdsStrLst(res.absentBaselines()));
+        }
+        else {
+            assert !res.emptyNodes().isEmpty();

Review Comment:
   what this assertion is used for ? Validates that thin client can\t return 
results while disconnected ? I suggest to remove it.



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";
+
+    /** */
+    public static final String UNSURED_DELETION_PREF = "WARNING: the following 
nodes found snapshot data but might not " +
+        "remove it completely ";
+
+    /** */
+    public static final String REMOVED_PREF = "Snapshot removal is completed 
on ";
+
+    /** */
+    public static final String NODE_NOT_FOUND_PREF = "NOTE: the following 
nodes can't find any snapshot data, " +
+        "operation skipped ";
+
+    /** */
+    public static final String NOT_FOUND_PREF = "Snapshot not found on current 
server nodes ";
+
+    /** */
+    public static final String MISSING_BASELINES = "WARNING: the snapshot's 
baseline nodes with the following consistent " +
+        "ids are missing in current cluster ";
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public String description() {
+        return DESC;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteCommandArg> argClass() {
+        return SnapshotDeleteCommandArg.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteTask> taskClass() {
+        return SnapshotDeleteTask.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public void printResult(SnapshotDeleteCommandArg arg, 
SnapshotDeleteProcessResult res, Consumer<String> printer) {
+        boolean found = false;
+
+        if (!res.uncompletedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(UNSURED_DELETION_PREF + 
nodeIdPairsStrLst(res.uncompletedNodes()));
+
+            printer.accept("");
+        }
+
+        if (!res.completedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(REMOVED_PREF + 
nodeIdPairsStrLst(res.completedNodes()));
+            printer.accept("");
+        }
+
+        if (found) {
+            if (!res.emptyNodes().isEmpty())
+                printer.accept(NODE_NOT_FOUND_PREF + 
nodeIdPairsStrLst(res.emptyNodes()));
+
+            if (!res.absentBaselines().isEmpty())
+                printer.accept(MISSING_BASELINES + 
nodeIdsStrLst(res.absentBaselines()));
+        }
+        else {
+            assert !res.emptyNodes().isEmpty();
+
+            printer.accept(NOT_FOUND_PREF);
+        }
+    }
+
+    /** */
+    private static String nodeIdPairsStrLst(Map<UUID, String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + uuids.entrySet().stream()
+            .map(e -> e.getValue() + " [uuid=" + e.getKey() + ']')
+            .collect(Collectors.joining(", "));
+    }
+
+    /** */
+    private static String nodeIdsStrLst(Collection<String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + String.join(", ", uuids);
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt(SnapshotDeleteCommandArg arg) {
+        return "This operation will completely remove snapshot: '" + 
arg.snapshotName() + "' and all its incrementals " +

Review Comment:
   ```suggestion
           return "This operation will completely remove snapshot: '" + 
arg.snapshotName() + "' and all its incremental snapshots." +
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommandArg.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.management.api.Argument;
+import org.apache.ignite.internal.management.api.Positional;
+
+/** */
+public class SnapshotDeleteCommandArg extends IgniteDataTransferObject {
+    /** */
+    private static final long serialVersionUID = 0;
+
+    /** */
+    @Order(0)
+    @Positional
+    @Argument(description = "Snapshot name")
+    String snapshotName;
+
+    /** */
+    @Order(1)
+    @Argument(example = "path", optional = true, description = "Path to 
snapshot location directory. If not specified " +
+        "or specified a relative path, the default snapshot configuration 
directory will be used")
+    String src;
+
+    /** */
+    public String snapshotName() {
+        return snapshotName;
+    }
+
+    /** */
+    public void snapshotName(String snapshotName) {
+        this.snapshotName = snapshotName;
+    }
+
+    /** */
+    public String src() {
+        return src;

Review Comment:
   can it return `null` if undefined ?



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";
+
+    /** */
+    public static final String UNSURED_DELETION_PREF = "WARNING: the following 
nodes found snapshot data but might not " +
+        "remove it completely ";
+
+    /** */
+    public static final String REMOVED_PREF = "Snapshot removal is completed 
on ";
+
+    /** */
+    public static final String NODE_NOT_FOUND_PREF = "NOTE: the following 
nodes can't find any snapshot data, " +
+        "operation skipped ";
+
+    /** */
+    public static final String NOT_FOUND_PREF = "Snapshot not found on current 
server nodes ";
+
+    /** */
+    public static final String MISSING_BASELINES = "WARNING: the snapshot's 
baseline nodes with the following consistent " +
+        "ids are missing in current cluster ";
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public String description() {
+        return DESC;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteCommandArg> argClass() {
+        return SnapshotDeleteCommandArg.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteTask> taskClass() {
+        return SnapshotDeleteTask.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public void printResult(SnapshotDeleteCommandArg arg, 
SnapshotDeleteProcessResult res, Consumer<String> printer) {
+        boolean found = false;
+
+        if (!res.uncompletedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(UNSURED_DELETION_PREF + 
nodeIdPairsStrLst(res.uncompletedNodes()));
+
+            printer.accept("");
+        }
+
+        if (!res.completedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(REMOVED_PREF + 
nodeIdPairsStrLst(res.completedNodes()));
+            printer.accept("");
+        }
+
+        if (found) {
+            if (!res.emptyNodes().isEmpty())
+                printer.accept(NODE_NOT_FOUND_PREF + 
nodeIdPairsStrLst(res.emptyNodes()));
+
+            if (!res.absentBaselines().isEmpty())
+                printer.accept(MISSING_BASELINES + 
nodeIdsStrLst(res.absentBaselines()));
+        }
+        else {
+            assert !res.emptyNodes().isEmpty();
+
+            printer.accept(NOT_FOUND_PREF);
+        }
+    }
+
+    /** */
+    private static String nodeIdPairsStrLst(Map<UUID, String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + uuids.entrySet().stream()
+            .map(e -> e.getValue() + " [uuid=" + e.getKey() + ']')
+            .collect(Collectors.joining(", "));
+    }
+
+    /** */
+    private static String nodeIdsStrLst(Collection<String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + String.join(", ", uuids);
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt(SnapshotDeleteCommandArg arg) {
+        return "This operation will completely remove snapshot: '" + 
arg.snapshotName() + "' and all its incrementals " +
+            "from all online server nodes." +

Review Comment:
   ```suggestion
               "from all available server nodes." +
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommandArg.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.management.api.Argument;
+import org.apache.ignite.internal.management.api.Positional;
+
+/** */
+public class SnapshotDeleteCommandArg extends IgniteDataTransferObject {
+    /** */
+    private static final long serialVersionUID = 0;
+
+    /** */
+    @Order(0)
+    @Positional
+    @Argument(description = "Snapshot name")
+    String snapshotName;
+
+    /** */
+    @Order(1)
+    @Argument(example = "path", optional = true, description = "Path to 
snapshot location directory. If not specified " +

Review Comment:
   ```suggestion
       @Argument(example = "path/to/directory", optional = true, description = 
"Path to snapshot location directory. If not specified " +
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommandArg.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.management.api.Argument;
+import org.apache.ignite.internal.management.api.Positional;
+
+/** */
+public class SnapshotDeleteCommandArg extends IgniteDataTransferObject {
+    /** */
+    private static final long serialVersionUID = 0;
+
+    /** */
+    @Order(0)
+    @Positional
+    @Argument(description = "Snapshot name")
+    String snapshotName;
+
+    /** */
+    @Order(1)
+    @Argument(example = "path", optional = true, description = "Path to 
snapshot location directory. If not specified " +
+        "or specified a relative path, the default snapshot configuration 
directory will be used")
+    String src;
+
+    /** */
+    public String snapshotName() {
+        return snapshotName;

Review Comment:
   can it return `null` if undefined ?



##########
modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java:
##########
@@ -0,0 +1,133 @@
+/*
+ * 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.ignite.internal.management.snapshot;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcess;
+import 
org.apache.ignite.internal.processors.cache.persistence.snapshot.SnapshotDeleteProcessResult;
+import 
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Snapshot deletion command.
+ *
+ * @see SupportedFeatureRegistry#SNAPSHOT_DELETE_FEATURE
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteCommand extends 
AbstractSnapshotCommand<SnapshotDeleteCommandArg, SnapshotDeleteProcessResult> {
+    /** */
+    public static final String DESC = "Deletes snapshot and all its 
incrementals from all the online server nodes";
+
+    /** */
+    public static final String UNSURED_DELETION_PREF = "WARNING: the following 
nodes found snapshot data but might not " +
+        "remove it completely ";
+
+    /** */
+    public static final String REMOVED_PREF = "Snapshot removal is completed 
on ";
+
+    /** */
+    public static final String NODE_NOT_FOUND_PREF = "NOTE: the following 
nodes can't find any snapshot data, " +
+        "operation skipped ";
+
+    /** */
+    public static final String NOT_FOUND_PREF = "Snapshot not found on current 
server nodes ";
+
+    /** */
+    public static final String MISSING_BASELINES = "WARNING: the snapshot's 
baseline nodes with the following consistent " +
+        "ids are missing in current cluster ";
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public String description() {
+        return DESC;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteCommandArg> argClass() {
+        return SnapshotDeleteCommandArg.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public Class<SnapshotDeleteTask> taskClass() {
+        return SnapshotDeleteTask.class;
+    }
+
+    /** {@inheritDoc} */
+    @Override public void printResult(SnapshotDeleteCommandArg arg, 
SnapshotDeleteProcessResult res, Consumer<String> printer) {
+        boolean found = false;
+
+        if (!res.uncompletedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(UNSURED_DELETION_PREF + 
nodeIdPairsStrLst(res.uncompletedNodes()));
+
+            printer.accept("");
+        }
+
+        if (!res.completedNodes().isEmpty()) {
+            found = true;
+
+            printer.accept(REMOVED_PREF + 
nodeIdPairsStrLst(res.completedNodes()));
+            printer.accept("");
+        }
+
+        if (found) {
+            if (!res.emptyNodes().isEmpty())
+                printer.accept(NODE_NOT_FOUND_PREF + 
nodeIdPairsStrLst(res.emptyNodes()));
+
+            if (!res.absentBaselines().isEmpty())
+                printer.accept(MISSING_BASELINES + 
nodeIdsStrLst(res.absentBaselines()));
+        }
+        else {
+            assert !res.emptyNodes().isEmpty();
+
+            printer.accept(NOT_FOUND_PREF);
+        }
+    }
+
+    /** */
+    private static String nodeIdPairsStrLst(Map<UUID, String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + uuids.entrySet().stream()
+            .map(e -> e.getValue() + " [uuid=" + e.getKey() + ']')
+            .collect(Collectors.joining(", "));
+    }
+
+    /** */
+    private static String nodeIdsStrLst(Collection<String> uuids) {
+        return "[cnt=" + uuids.size() + "]: " + String.join(", ", uuids);
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt(SnapshotDeleteCommandArg arg) {
+        return "This operation will completely remove snapshot: '" + 
arg.snapshotName() + "' and all its incrementals." +
+            U.nl() + U.nl() +
+            "If the security is enabled, the operation requires the snapshot 
administration permissions." +
+            U.nl() + U.nl() +
+            "Deletion in not snapshots Ignite's directories and deletion of 
any data without or corrupted snapshot " +

Review Comment:
   ```suggestion
               "Deletion of any data without (valid) snapshot metadata is 
prohibited." +
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -701,46 +708,108 @@ public IgniteSnapshotManager(GridKernalContext ctx) {
     /**
      * @param snpDir Snapshot dir.
      */
-    public void deleteSnapshot(File snpDir) {
+    public void deleteLocalSnapshot(File snpDir) {
         if (!snpDir.exists())
             return;
 
         if (!snpDir.isDirectory())
             return;
 
-        deleteSnapshot(new SnapshotFileTree(
+        var sft = new SnapshotFileTree(
             cctx.kernalContext(),
             snpDir.getName(),
             snpDir.getParent(),
             ft.folderName(),
-            pdsSettings.consistentId().toString()));
+            pdsSettings.consistentId().toString()
+        );
+
+        deleteLocalSnapshot(sft, null);
     }
 
-    /** */
-    public void deleteSnapshot(SnapshotFileTree sft) {
+    /**
+     * Tries to delete local snapshot data.
+     *
+     * @param sft Snapshot file tree.
+     * @param existsFlag Flag to set {@code true} if any snapshot file or 
directory was found (existed). If {@code null}, ignored.
+     * @return {@code True}, if data is found and completely deleted;
+     *         {@code False}, if nothing found or if data is found but might 
not be deleted completely.
+     */
+    public boolean deleteLocalSnapshot(SnapshotFileTree sft, @Nullable 
AtomicBoolean existsFlag) {

Review Comment:
   I wonder - why Atomic here ?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -701,46 +708,108 @@ public IgniteSnapshotManager(GridKernalContext ctx) {
     /**
      * @param snpDir Snapshot dir.
      */
-    public void deleteSnapshot(File snpDir) {
+    public void deleteLocalSnapshot(File snpDir) {
         if (!snpDir.exists())
             return;
 
         if (!snpDir.isDirectory())
             return;
 
-        deleteSnapshot(new SnapshotFileTree(
+        var sft = new SnapshotFileTree(
             cctx.kernalContext(),
             snpDir.getName(),
             snpDir.getParent(),
             ft.folderName(),
-            pdsSettings.consistentId().toString()));
+            pdsSettings.consistentId().toString()
+        );
+
+        deleteLocalSnapshot(sft, null);
     }
 
-    /** */
-    public void deleteSnapshot(SnapshotFileTree sft) {
+    /**
+     * Tries to delete local snapshot data.
+     *
+     * @param sft Snapshot file tree.
+     * @param existsFlag Flag to set {@code true} if any snapshot file or 
directory was found (existed). If {@code null}, ignored.
+     * @return {@code True}, if data is found and completely deleted;
+     *         {@code False}, if nothing found or if data is found but might 
not be deleted completely.
+     */
+    public boolean deleteLocalSnapshot(SnapshotFileTree sft, @Nullable 
AtomicBoolean existsFlag) {
+        var exFlag0 = new AtomicBoolean();
+
+        sft.allStorages().forEach(s -> {
+            if (s.exists())
+                exFlag0.set(true);
+        });
+
+        if (sft.root().exists())
+            exFlag0.set(true);
+
+        if (existsFlag != null)
+            existsFlag.set(exFlag0.get());
+
+        // Nothing to delete.
+        if (!exFlag0.get())
+            return false;
+
+        boolean res = true;
+
         try {
-            U.delete(sft.binaryMeta());
-            sft.allStorages().forEach(U::delete);
-            U.delete(sft.meta());
+            if (sft.binaryMeta().exists() && !U.delete(sft.binaryMeta()) && 
sft.binaryMeta().exists())
+                res = false;

Review Comment:
   why not simple return ? 



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -701,46 +708,108 @@ public IgniteSnapshotManager(GridKernalContext ctx) {
     /**
      * @param snpDir Snapshot dir.
      */
-    public void deleteSnapshot(File snpDir) {
+    public void deleteLocalSnapshot(File snpDir) {
         if (!snpDir.exists())
             return;
 
         if (!snpDir.isDirectory())
             return;
 
-        deleteSnapshot(new SnapshotFileTree(
+        var sft = new SnapshotFileTree(
             cctx.kernalContext(),
             snpDir.getName(),
             snpDir.getParent(),
             ft.folderName(),
-            pdsSettings.consistentId().toString()));
+            pdsSettings.consistentId().toString()
+        );
+
+        deleteLocalSnapshot(sft, null);
     }
 
-    /** */
-    public void deleteSnapshot(SnapshotFileTree sft) {
+    /**
+     * Tries to delete local snapshot data.
+     *
+     * @param sft Snapshot file tree.
+     * @param existsFlag Flag to set {@code true} if any snapshot file or 
directory was found (existed). If {@code null}, ignored.
+     * @return {@code True}, if data is found and completely deleted;
+     *         {@code False}, if nothing found or if data is found but might 
not be deleted completely.
+     */
+    public boolean deleteLocalSnapshot(SnapshotFileTree sft, @Nullable 
AtomicBoolean existsFlag) {
+        var exFlag0 = new AtomicBoolean();
+
+        sft.allStorages().forEach(s -> {
+            if (s.exists())
+                exFlag0.set(true);
+        });
+
+        if (sft.root().exists())
+            exFlag0.set(true);
+
+        if (existsFlag != null)
+            existsFlag.set(exFlag0.get());
+
+        // Nothing to delete.
+        if (!exFlag0.get())
+            return false;
+
+        boolean res = true;
+
         try {
-            U.delete(sft.binaryMeta());
-            sft.allStorages().forEach(U::delete);
-            U.delete(sft.meta());
+            if (sft.binaryMeta().exists() && !U.delete(sft.binaryMeta()) && 
sft.binaryMeta().exists())

Review Comment:
   ```suggestion
               if (sft.binaryMeta().exists() && !U.delete(sft.binaryMeta()))
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -701,46 +708,108 @@ public IgniteSnapshotManager(GridKernalContext ctx) {
     /**
      * @param snpDir Snapshot dir.
      */
-    public void deleteSnapshot(File snpDir) {
+    public void deleteLocalSnapshot(File snpDir) {
         if (!snpDir.exists())
             return;
 
         if (!snpDir.isDirectory())
             return;
 
-        deleteSnapshot(new SnapshotFileTree(
+        var sft = new SnapshotFileTree(
             cctx.kernalContext(),
             snpDir.getName(),
             snpDir.getParent(),
             ft.folderName(),
-            pdsSettings.consistentId().toString()));
+            pdsSettings.consistentId().toString()
+        );
+
+        deleteLocalSnapshot(sft, null);
     }
 
-    /** */
-    public void deleteSnapshot(SnapshotFileTree sft) {
+    /**
+     * Tries to delete local snapshot data.
+     *
+     * @param sft Snapshot file tree.
+     * @param existsFlag Flag to set {@code true} if any snapshot file or 
directory was found (existed). If {@code null}, ignored.
+     * @return {@code True}, if data is found and completely deleted;
+     *         {@code False}, if nothing found or if data is found but might 
not be deleted completely.
+     */
+    public boolean deleteLocalSnapshot(SnapshotFileTree sft, @Nullable 
AtomicBoolean existsFlag) {
+        var exFlag0 = new AtomicBoolean();
+
+        sft.allStorages().forEach(s -> {
+            if (s.exists())
+                exFlag0.set(true);
+        });
+
+        if (sft.root().exists())
+            exFlag0.set(true);
+
+        if (existsFlag != null)
+            existsFlag.set(exFlag0.get());
+
+        // Nothing to delete.
+        if (!exFlag0.get())
+            return false;
+
+        boolean res = true;
+
         try {
-            U.delete(sft.binaryMeta());
-            sft.allStorages().forEach(U::delete);
-            U.delete(sft.meta());
+            if (sft.binaryMeta().exists() && !U.delete(sft.binaryMeta()) && 
sft.binaryMeta().exists())
+                res = false;
+
+            for (var s : sft.allStorages().toList()) {
+                if (s.exists() && !U.delete(s) && s.exists())
+                    res = false;
+            }
 
-            deleteDirectory(sft.binaryMetaRoot());
-            deleteDirectory(sft.marshaller());
+            if (sft.meta().exists() && !U.delete(sft.meta()) && 
sft.meta().exists())

Review Comment:
   from spec i can found that : (translated ver)
   
   `All data in the snapshot directories is deleted. There are no checks on 
their type or affiliation to the snapshot.`
   
   thus - why do you need all near checks ?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotOperationRequest.java:
##########
@@ -79,7 +79,7 @@ protected AbstractSnapshotOperationRequest(
         Collection<UUID> nodes
     ) {
         this.reqId = reqId;
-        this.snpName = snpName;
+        this.snpName = snpName.trim();

Review Comment:
   why do you need this change here ? It need to be fixed after parsing cli 
params.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to