Copilot commented on code in PR #13577:
URL: https://github.com/apache/ignite/pull/13577#discussion_r4083428371
##########
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())
+ res = false;
+
+ if (sft.binaryMetaRoot().exists() &&
!deleteDirectory(sft.binaryMetaRoot()) && sft.binaryMetaRoot().exists())
+ res = false;
+
+ if (sft.marshaller().exists() &&
!deleteDirectory(sft.marshaller()) && sft.marshaller().exists())
+ res = false;
+
+ if (sft.incrementsRoot().exists() &&
!deleteDirectory(sft.incrementsRoot()) && sft.incrementsRoot().exists())
+ res = false;
// Delete parent dir which is {snapshot_root}/db if empty.
- sft.marshaller().getParentFile().delete();
+ if (!sft.marshaller().getParentFile().delete() &&
sft.marshaller().getParentFile().exists())
+ res = false;
+
// Delete root dir which is {snapshot_root} if empty.
- sft.root().delete();
+ if (!sft.root().delete() && sft.root().exists())
+ res = false;
}
- catch (IOException e) {
- throw new IgniteException(e);
+ catch (Exception e) {
+ log.warning("Failed to delete local snapshot [snpName=" +
sft.name() + ']', e);
+
+ return false;
}
+
+ for (var s : sft.allStorages().toList()) {
+ if (s.exists())
+ return false;
+ }
+
+ if (sft.root().exists())
+ return false;
+
+ return res;
}
/** Concurrently traverse the directory and delete all files. */
- private void deleteDirectory(File dir) throws IOException {
- Files.walkFileTree(dir.toPath(), new SimpleFileVisitor<Path>() {
+ private boolean deleteDirectory(File dir) throws IOException {
+ var res = new AtomicBoolean();
+
+ Files.walkFileTree(dir.toPath(), new SimpleFileVisitor<>() {
@Override public FileVisitResult visitFile(Path file,
BasicFileAttributes attrs) {
- U.delete(file);
+ File f0 = file.toFile();
+
+ if (f0.exists() && !U.delete(f0) && f0.exists())
+ res.set(false);
return FileVisitResult.CONTINUE;
}
Review Comment:
`deleteDirectory` currently returns `false` even on success because `res` is
initialized to `false` and never set to `true`. This makes
`deleteLocalSnapshot(...)` mark directory deletions as failed even when they
succeed. Initialize `res` to `true` (and only flip to `false` on failures), or
invert the flag semantics (e.g., `failed` initialized to `false`).
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -751,14 +820,19 @@ private void deleteDirectory(File dir) throws IOException
{
}
@Override public FileVisitResult postVisitDirectory(Path dir,
IOException e) {
- dir.toFile().delete();
+ File f0 = dir.toFile();
+
+ if (f0.exists() && !f0.delete() && f0.exists())
+ res.set(false);
Review Comment:
`deleteDirectory` currently returns `false` even on success because `res` is
initialized to `false` and never set to `true`. This makes
`deleteLocalSnapshot(...)` mark directory deletions as failed even when they
succeed. Initialize `res` to `true` (and only flip to `false` on failures), or
invert the flag semantics (e.g., `failed` initialized to `false`).
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -751,14 +820,19 @@ private void deleteDirectory(File dir) throws IOException
{
}
@Override public FileVisitResult postVisitDirectory(Path dir,
IOException e) {
- dir.toFile().delete();
+ File f0 = dir.toFile();
+
+ if (f0.exists() && !f0.delete() && f0.exists())
+ res.set(false);
if (log.isInfoEnabled() && e != null)
log.info("Snapshot directory cleaned with an exception
[dir=" + dir + ", e=" + e.getMessage() + ']');
return FileVisitResult.CONTINUE;
}
});
+
+ return res.get();
}
Review Comment:
`deleteDirectory` currently returns `false` even on success because `res` is
initialized to `false` and never set to `true`. This makes
`deleteLocalSnapshot(...)` mark directory deletions as failed even when they
succeed. Initialize `res` to `true` (and only flip to `false` on failures), or
invert the flag semantics (e.g., `failed` initialized to `false`).
##########
docs/_docs/snapshots/snapshots.adoc:
##########
@@ -287,6 +287,52 @@ 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 (create, restore, check,
delete) is active for the snapshot.
+* The operation requires the snapshot administration pesmissins via
`IgniteSecurity` (if configured).
+* 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.
+* If nodes share the same snapshot directory, a deletion concurrency may
apper. Some nodes may not find snapshot at all
+or decide that the removal wasn't complete. Snapshot is deleted anyway.
+* The removal operation is not subject for the snapshot operation status and
cancel requests.
Review Comment:
Typos/grammar in documentation: “pesmissins” → “permissions”, “a not
snapshot” → “a non-snapshot”, “apper” → “appear”, “not subject for” → “not
subject to”, and line 332–333 should be reflowed/clarified. These are public
docs and should be corrected.
##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerDeleteSnapshotTest.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.ignite.util;
+
+import java.io.File;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collection;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.management.snapshot.SnapshotDeleteCommand;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.junit.Test;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+import static java.nio.file.Files.newDirectoryStream;
+import static org.apache.ignite.cluster.ClusterState.ACTIVE;
+import static
org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_OK;
+import static
org.apache.ignite.internal.processors.cache.persistence.snapshot.AbstractSnapshotSelfTest.snp;
+import static org.apache.ignite.testframework.GridTestUtils.waitForCondition;
+import static org.junit.Assume.assumeTrue;
+
+/** Test for the command '--snapshot delete'. */
+public class GridCommandHandlerDeleteSnapshotTest extends
GridCommandHandlerAbstractTest {
+ /** Value: -1 - do not use, 1 - server node, 0 - client node. */
+ @Parameter(1)
+ public int extraNodeIsServer = -1;
+
+ /** */
+ @Parameter(2)
+ public boolean incremental;
+
+ /** */
+ @Parameter(3)
+ public boolean changeBaseline;
+
+ /** */
+ @Parameter(4)
+ public boolean customPath;
+
+ /** */
+ @Parameter(5)
+ public boolean separatedWorkDir;
+
+ /** */
+ @Parameters(name =
"client={0},useExtraNode={1},inc={2},chBaseln={3},cstSnpPath={4},ownWorkDir={5}")
+ public static Collection<?> parameters() {
+ return GridTestUtils.cartesianProduct(
+ commandHandlers(),
+ F.asList(-1, 1, 0), // Use extra node (do not use at all, server
node, client node);
+ F.asList(false, true), // Add incremental snapshot;
+ F.asList(false, true), // Change baseline;
+ F.asList(false, true), // Use custom snapshot path;
+ F.asList(false, true) // Separated (own) work directory.
+ );
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ super.afterTest();
+
+ stopAllGrids();
+
+ cleanPersistenceDir();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ super.beforeTest();
+
+ /** Handy if test running is interrupted and {@link #afterTest()}
isn't invoked. */
+ cleanPersistenceDir();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void cleanPersistenceDir() throws Exception {
+ super.cleanPersistenceDir();
+
+ // Also cleans separated snapshot working directories and custom
snapshot patches.
+ try (DirectoryStream<Path> files =
newDirectoryStream(Paths.get(U.defaultWorkDirectory()))) {
+ for (Path path : files)
+ U.delete(path);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String
igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ if (separatedWorkDir)
+ cfg.setWorkDirectory(new File(U.defaultWorkDirectory(),
igniteInstanceName).getAbsolutePath());
+
+ return cfg;
+ }
+
+ /** */
+ @Test
+ public void testSnapshotDelete() throws Exception {
+ // A custom snapshot path actually puts snapshots in a shared
directory. This skews the results when dedicated
+ // work directories are set.
+ assumeTrue(!customPath || !separatedWorkDir);
+
+ int entriesCnt = 100;
+ int initNodes = 3;
+
+ walCompactionEnabled(incremental);
+
+ IgniteEx ig = (IgniteEx)startGridsMultiThreaded(initNodes);
+
+ if (changeBaseline) {
+ ig.cluster().baselineAutoAdjustEnabled(false);
+
+ ig.cluster().setBaselineTopology(ig.cluster().topologyVersion());
+ }
+
+ ig.cluster().state(ACTIVE);
+
+ createCacheAndPreload(ig, entriesCnt);
+
+ File cstSnpsRoot = customPath
+ ? new
File(grid(0).context().pdsFolderResolver().fileTree().snapshotsRoot(),
"ex_snapshots")
+ : null;
+ File snpDir = new File(customPath ? cstSnpsRoot :
ig.context().pdsFolderResolver().fileTree().snapshotsRoot(), "testSnapshot");
+
+ snp(ig).createSnapshot("testSnapshot", customPath ?
cstSnpsRoot.getAbsolutePath() : null, false, false)
+ .get(getTestTimeout());
+
+ if (incremental) {
+ for (int i = 0; i < 3; ++i) {
+ int dataIdx = entriesCnt + entriesCnt / 4 * i;
+
+ try (IgniteDataStreamer<Object, Object> streamer =
ig.dataStreamer(DEFAULT_CACHE_NAME)) {
+ for (int d = dataIdx; d < dataIdx + entriesCnt / 4; ++d)
+ streamer.addData(i, i);
Review Comment:
The loop variable `d` is not used when streaming data; `streamer.addData(i,
i)` repeatedly writes the same key/value instead of changing the dataset. This
likely undermines the intent of generating new data for incremental snapshots.
Use `d` as the key/value (or otherwise vary keys) so each iteration adds
distinct entries.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotDeleteRequest.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.processors.cache.persistence.snapshot;
+
+import java.io.File;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Cluster snapshot delete distributed process request.
+ *
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteRequest implements Message {
+ /** Request ID. */
+ @Order(0)
+ UUID reqId;
+
+ /** Snapshot name. */
+ @Order(1)
+ String snpName;
+
+ /** Snapshot directory path. */
+ @Order(2)
+ @Nullable String snpPath;
+
+ /** Resolved absolute path. Transient */
+ @GridToStringExclude
+ @Nullable File resolvedPath;
+
+ /** Default constructor for {@link MessageFactory}. */
+ public SnapshotDeleteRequest() {
+ // No-op.
+ }
+
+ /**
+ * @param reqId Request ID.
+ * @param snpName Snapshot name.
+ * @param snpPath Snapshot directory path.
+ */
+ SnapshotDeleteRequest(UUID reqId, String snpName, @Nullable String
snpPath) {
+ this.reqId = reqId;
+ this.snpName = snpName.trim();
Review Comment:
Two issues here: (1) `toLowerCase()` without an explicit locale can produce
surprising results in some locales (e.g., Turkish). Use
`toLowerCase(Locale.ROOT)` for stable hashing. (2) equality/hashing relies on
`File.equals()` for `resolvedPath`, which is string-path based and not
normalized; semantically equivalent paths (trailing separator, `..`, symlinks)
may bypass the “already started” guard. Consider normalizing/canonicalizing the
resolved path before storing/using it for equality.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotDeleteRequest.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.processors.cache.persistence.snapshot;
+
+import java.io.File;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Cluster snapshot delete distributed process request.
+ *
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteRequest implements Message {
+ /** Request ID. */
+ @Order(0)
+ UUID reqId;
+
+ /** Snapshot name. */
+ @Order(1)
+ String snpName;
+
+ /** Snapshot directory path. */
+ @Order(2)
+ @Nullable String snpPath;
+
+ /** Resolved absolute path. Transient */
+ @GridToStringExclude
+ @Nullable File resolvedPath;
+
+ /** Default constructor for {@link MessageFactory}. */
+ public SnapshotDeleteRequest() {
+ // No-op.
+ }
+
+ /**
+ * @param reqId Request ID.
+ * @param snpName Snapshot name.
+ * @param snpPath Snapshot directory path.
+ */
+ SnapshotDeleteRequest(UUID reqId, String snpName, @Nullable String
snpPath) {
+ this.reqId = reqId;
+ this.snpName = snpName.trim();
+
+ // A protection against empty relative paths like " ".
+ if (!F.isEmpty(snpPath))
+ snpPath = snpPath.trim();
+
+ snpPath = F.isEmpty(snpPath) ? null : snpPath;
+
+ this.snpPath = snpPath;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ SnapshotDeleteRequest other = (SnapshotDeleteRequest)o;
+
+ return snpName.equalsIgnoreCase(other.snpName) &&
Objects.equals(resolvedPath, other.resolvedPath);
+ }
+
+ /** {@inheritDoc} */
+ @Override public int hashCode() {
+ return Objects.hash(snpName.toLowerCase(), resolvedPath);
+ }
Review Comment:
Two issues here: (1) `toLowerCase()` without an explicit locale can produce
surprising results in some locales (e.g., Turkish). Use
`toLowerCase(Locale.ROOT)` for stable hashing. (2) equality/hashing relies on
`File.equals()` for `resolvedPath`, which is string-path based and not
normalized; semantically equivalent paths (trailing separator, `..`, symlinks)
may bypass the “already started” guard. Consider normalizing/canonicalizing the
resolved path before storing/using it for equality.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotDeleteRequest.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.processors.cache.persistence.snapshot;
+
+import java.io.File;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Cluster snapshot delete distributed process request.
+ *
+ * @see SnapshotDeleteProcess
+ */
+public class SnapshotDeleteRequest implements Message {
+ /** Request ID. */
+ @Order(0)
+ UUID reqId;
+
+ /** Snapshot name. */
+ @Order(1)
+ String snpName;
+
+ /** Snapshot directory path. */
+ @Order(2)
+ @Nullable String snpPath;
+
+ /** Resolved absolute path. Transient */
+ @GridToStringExclude
+ @Nullable File resolvedPath;
+
+ /** Default constructor for {@link MessageFactory}. */
+ public SnapshotDeleteRequest() {
+ // No-op.
+ }
+
+ /**
+ * @param reqId Request ID.
+ * @param snpName Snapshot name.
+ * @param snpPath Snapshot directory path.
+ */
+ SnapshotDeleteRequest(UUID reqId, String snpName, @Nullable String
snpPath) {
+ this.reqId = reqId;
+ this.snpName = snpName.trim();
+
+ // A protection against empty relative paths like " ".
+ if (!F.isEmpty(snpPath))
+ snpPath = snpPath.trim();
+
+ snpPath = F.isEmpty(snpPath) ? null : snpPath;
+
+ this.snpPath = snpPath;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ SnapshotDeleteRequest other = (SnapshotDeleteRequest)o;
+
+ return snpName.equalsIgnoreCase(other.snpName) &&
Objects.equals(resolvedPath, other.resolvedPath);
+ }
Review Comment:
Two issues here: (1) `toLowerCase()` without an explicit locale can produce
surprising results in some locales (e.g., Turkish). Use
`toLowerCase(Locale.ROOT)` for stable hashing. (2) equality/hashing relies on
`File.equals()` for `resolvedPath`, which is string-path based and not
normalized; semantically equivalent paths (trailing separator, `..`, symlinks)
may bypass the “already started” guard. Consider normalizing/canonicalizing the
resolved path before storing/using it for equality.
##########
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 ";
Review Comment:
The identifier and the output text contain a typo/awkward phrasing:
`UNSURED_DELETION_PREF` should be `UNSURE...` (or similar), and the message
would read better with punctuation at the end (e.g., a colon/period) rather
than a trailing space.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotRestoreProcess.java:
##########
@@ -659,6 +659,9 @@ private
IgniteInternalFuture<SnapshotRestoreOperationResponse> prepare(UUID igno
if (snpMgr.isSnapshotCreating())
throw new IgniteCheckedException(OP_REJECT_MSG + "A cluster
snapshot operation is in progress.");
+ if (snpMgr.isSnapshotDeleting(req.snapshotName(),
req.snapshotPath()))
+ throw new IgniteException(OP_REJECT_MSG + "A snapshot '" +
req.snapshotName() + "' delete operation is in progress.");
Review Comment:
This method rejects other snapshot states via `IgniteCheckedException`, but
the new “delete in progress” branch throws `IgniteException` (unchecked). That
can change error propagation/handling relative to other rejection paths. Prefer
throwing `IgniteCheckedException` here for consistency with surrounding
validation logic and expected failure type.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotDeleteProcess.java:
##########
@@ -0,0 +1,425 @@
+/*
+ * 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.processors.cache.persistence.snapshot;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.ignite.IgniteIllegalStateException;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.GridKernalContext;
+import org.apache.ignite.internal.IgniteInternalFuture;
+import org.apache.ignite.internal.NodeStoppingException;
+import
org.apache.ignite.internal.processors.cache.persistence.filename.SnapshotFileTree;
+import org.apache.ignite.internal.util.CommonUtils;
+import org.apache.ignite.internal.util.distributed.DistributedProcess;
+import org.apache.ignite.internal.util.future.GridCompoundFuture;
+import org.apache.ignite.internal.util.future.GridFinishedFuture;
+import org.apache.ignite.internal.util.future.GridFutureAdapter;
+import org.apache.ignite.internal.util.future.IgniteFutureImpl;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteReducer;
+import org.jetbrains.annotations.Nullable;
+
+import static
org.apache.ignite.internal.processors.rollingupgrade.feature.SupportedFeatureRegistry.SNAPSHOT_DELETE_FEATURE;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.DELETE_SNAPSHOT;
+import static
org.apache.ignite.plugin.security.SecurityPermission.ADMIN_SNAPSHOT;
+
+/**
+ * Distributed process to delete a cluster snapshot. The operation is rejected
if any concurrent snapshot operation is
+ * active.
+ */
+public class SnapshotDeleteProcess {
+ /** Reject operation messages. */
+ private static final String OP_REJECT_MSG = "Snapshot deletion was
rejected. ";
+
+ /** */
+ private static final String SNP_PATH_ERR_PREF = "Provided snapshot path ";
+
+ /** Kernal context. */
+ private final GridKernalContext kctx;
+
+ /** Logger. */
+ private final IgniteLogger log;
+
+ /** */
+ private volatile boolean interrupted;
+
+ /** Cluster-wide operation futures per request id on certain node. */
+ private final Map<UUID, GridFutureAdapter<SnapshotDeleteProcessResult>>
clusterOpFuts = new ConcurrentHashMap<>();
+
+ /** Process requests per snapshot name on each server node. */
+ private final Set<SnapshotDeleteRequest> requests =
ConcurrentHashMap.newKeySet();
+
+ /** The distributed process. */
+ private final DistributedProcess<SnapshotDeleteRequest,
SnapshotDeleteResponse> distrProc;
+
+ /**
+ * @param ctx Kernal context.
+ */
+ public SnapshotDeleteProcess(GridKernalContext ctx) {
+ this.kctx = ctx;
+
+ log = ctx.log(getClass());
+
+ distrProc = new DistributedProcess<>(ctx, DELETE_SNAPSHOT,
this::deletePhase, this::reducePhase);
+ }
+
+ /**
+ * Starts the cluster snapshot delete process.
+ *
+ * @param snpName Snapshot name.
+ * @param snpPath Snapshot directory path (optional).
+ * @return Future that will be completed when the snapshot is deleted.
+ */
+ public IgniteFuture<SnapshotDeleteProcessResult> start(String snpName,
@Nullable String snpPath) {
+ var clusterOpFut = new
GridFutureAdapter<SnapshotDeleteProcessResult>();
+
+ if
(!kctx.rollingUpgrade().features().isActive(SNAPSHOT_DELETE_FEATURE)) {
+ clusterOpFut.onDone(new IgniteIllegalStateException(OP_REJECT_MSG +
+ "The snapshot deletion feature isn't activated yet [snpName="
+ snpName + ", snpPath=" + snpPath + ']'));
+
+ return new IgniteFutureImpl<>(clusterOpFut);
+ }
+
+ UUID reqId = UUID.randomUUID();
+
+ clusterOpFut.listen(fut -> clusterOpFuts.remove(reqId));
+
+ try {
+ synchronized (clusterOpFuts) {
+ if (interrupted || kctx.isStopping())
+ throw new NodeStoppingException("Failed to start snapshot
delete process: node is stopping.");
+
+ clusterOpFuts.put(reqId, clusterOpFut);
+ }
+
+ SnapshotDeleteRequest req = new SnapshotDeleteRequest(reqId,
snpName, snpPath);
+
+ distrProc.start(reqId, req);
+ }
+ catch (Throwable t) {
+ log.error("Failed to start distributed delete snapshot process
[snpName=" + snpName + ", snpPath=" + snpPath + ']', t);
+
+ clusterOpFut.onDone(t);
+ }
+
+ return new IgniteFutureImpl<>(clusterOpFut);
+ }
+
+ /** */
+ private IgniteInternalFuture<SnapshotDeleteResponse> deletePhase(UUID
ignored, SnapshotDeleteRequest req) {
+ if (interrupted || kctx.isStopping()) {
+ return new GridFinishedFuture<>(new
NodeStoppingException(OP_REJECT_MSG +
+ " Node is stopping [req=" + req + ']'));
+ }
+
+ if (kctx.cluster().get().localNode().isClient())
+ return new GridFinishedFuture<>(new SnapshotDeleteResponse());
+
+ kctx.security().authorize(ADMIN_SNAPSHOT);
+
+ IgniteSnapshotManager snpMgr = kctx.cache().context().snapshotMgr();
+
+ var curCreateRq = snpMgr.currentCreateRequest();
+
+ if (curCreateRq != null &&
curCreateRq.snpName.equalsIgnoreCase(req.snpName)) {
+ return new GridFinishedFuture<>(new
IgniteIllegalStateException(OP_REJECT_MSG +
+ "Snapshot with this name is being created [req=" + req + ']'));
+ }
+
+ if (snpMgr.isRestoring(req.snpName)) {
+ return new GridFinishedFuture<>(new
IgniteIllegalStateException(OP_REJECT_MSG +
+ "Snapshot with this name is being restored [req=" + req +
']'));
+ }
+
+ if (snpMgr.isSnapshotChecking(req.snpName)) {
+ return new GridFinishedFuture<>(new
IgniteIllegalStateException(OP_REJECT_MSG +
+ "Snapshot with this name is being checked [req=" + req + ']'));
+ }
+
+ File path = resolvePath(req.snpPath);
+
+ String pathValidationErr = validateAbsoluteSnapshotRoot(path);
+
+ if (pathValidationErr != null) {
+ return new GridFinishedFuture<>(new
IllegalArgumentException(OP_REJECT_MSG +
+ SNP_PATH_ERR_PREF + pathValidationErr + " [req=" + req + ']'));
+ }
+
+ req.resolvedPath = path;
+
+ try {
+ if (!requests.add(req)) {
+ return new GridFinishedFuture<>(new
IgniteIllegalStateException("Deletion of the snapshot has already " +
+ "started [req=" + req + ']'));
+ }
+
+ SnapshotFileTree snpFiles = new SnapshotFileTree(kctx,
req.snpName, path.getAbsolutePath());
+
+ // We need to find and read snapshot metas to ensure the content
is a snapshot. Also, the metas contain
+ // initial cluster topology and actual snasphot folder names.
+ List<SnapshotMetadata> locMetas =
kctx.cache().context().snapshotMgr().readSnapshotMetadatas(snpFiles, false);
+
+ if (locMetas.isEmpty()) {
+ requests.remove(req);
+
+ log.warning("Snapshot deletion won't process, no snapshot
metadata found [req=" + req + ']');
+
+ return new GridFinishedFuture<>(new
SnapshotDeleteResponse(SnapshotDeleteResponse.DeleteStatus.NOT_FOUND, null));
+ }
+
+ // Future to delete snapshot contents according to snapshot
metadatas.
+ GridCompoundFuture<SnapshotDeleteResponse, SnapshotDeleteResponse>
resultFut =
+ new GridCompoundFuture<>(new MetaFuturesReducer());
+
+ resultFut.listen(fut -> requests.remove(req));
+
+ File path0 = path;
+
+ for (var meta : locMetas) {
+ GridFutureAdapter<SnapshotDeleteResponse> perMetaFut = new
GridFutureAdapter<>();
+
+ kctx.pools().getSnapshotExecutorService().submit(() -> {
+ try {
+ AtomicBoolean foundFlag = new AtomicBoolean();
+
+ // Read file tree of the snapshot.
+ var byMetaSft = new SnapshotFileTree(kctx,
req.snpName, path0.getAbsolutePath(), meta.folderName(),
+ meta.consId);
+
+ boolean deleted =
snpMgr.deleteLocalSnapshot(byMetaSft, foundFlag);
+
+ SnapshotDeleteResponse.DeleteStatus status;
+
+ if (foundFlag.get()) {
+ if (deleted && log.isInfoEnabled())
+ log.info("Snapshot successfully deleted [req="
+ req + ']');
+ else if (!deleted)
+ log.warning("Snapshot deleted not completely
[req=" + req + ']');
+
+ status = deleted
+ ? SnapshotDeleteResponse.DeleteStatus.DELETED
+ : SnapshotDeleteResponse.DeleteStatus.PARTLY;
+ }
+ else {
+ if (log.isInfoEnabled())
+ log.info("Snapshot not found to delete [req="
+ req + ']');
+
+ status =
SnapshotDeleteResponse.DeleteStatus.NOT_FOUND;
+ }
+
+ perMetaFut.onDone(new SnapshotDeleteResponse(status,
meta.bltNodes));
+ }
+ catch (Throwable e) {
+ perMetaFut.onDone(e);
+ }
+ });
+
+ resultFut.add(perMetaFut);
+ }
+
+ resultFut.markInitialized();
+
+ if (log.isInfoEnabled())
+ log.info("Deletion of snapshot initialized [req=" + req + ']');
+
+ return resultFut;
+ }
+ catch (Throwable t) {
+ requests.remove(req);
+
+ log.warning("An error occurred during snapshot deletion [req=" +
req + ']', t);
+
+ return new GridFinishedFuture<>(t);
+ }
+ }
+
+ /** */
+ private File resolvePath(@Nullable String path) {
+ var res = kctx.pdsFolderResolver().fileTree().snapshotsRoot();
+
+ if (path != null) {
+ File reqPath = new File(path);
+
+ res = reqPath.isAbsolute() ? reqPath : new File(res, path);
+ }
+
+ return res;
+ }
+
+ /** */
+ private @Nullable String validateAbsoluteSnapshotRoot(@Nullable File path)
{
+ if (path == null)
+ return null;
+
+ assert path.isAbsolute();
+
+ var ignFileTree = kctx.pdsFolderResolver().fileTree();
+
+ if (ignFileTree.snapshotsRoot().compareTo(path) != 0 &&
!contains(ignFileTree.snapshotsRoot(), path)) {
+ for (var ignPath : List.of(new File(CommonUtils.getIgniteHome()),
ignFileTree.root())) {
+ if (ignPath.compareTo(path) == 0 || contains(ignPath, path))
+ return "belongs to a an Ignite's directory";
Review Comment:
The returned validation text has a grammar issue (“a an”) and reads
awkwardly for a user-facing error. Consider changing it to something like
“belongs to an Ignite directory” (or “belongs to an Ignite work directory”) for
clarity. (Note: tests currently assert this message substring, so they’ll need
updating accordingly.)
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotDeleteTest.java:
##########
@@ -0,0 +1,775 @@
+/*
+ * 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.processors.cache.persistence.snapshot;
+
+import java.io.File;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Random;
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
+import org.apache.ignite.IgniteIllegalStateException;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.TestRecordingCommunicationSpi;
+import org.apache.ignite.internal.processors.cache.persistence.file.FileIO;
+import
org.apache.ignite.internal.processors.cache.persistence.file.RandomAccessFileIOFactory;
+import
org.apache.ignite.internal.processors.cache.persistence.filename.SnapshotFileTree;
+import org.apache.ignite.internal.util.CommonUtils;
+import org.apache.ignite.internal.util.distributed.DistributedProcess;
+import org.apache.ignite.internal.util.distributed.SingleNodeMessage;
+import org.apache.ignite.internal.util.future.IgniteFutureImpl;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.plugin.AbstractTestPluginProvider;
+import org.apache.ignite.plugin.PluginContext;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.CHECK_SNAPSHOT_METAS;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.CHECK_SNAPSHOT_PARTS;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.DELETE_SNAPSHOT;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.END_SNAPSHOT;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.RESTORE_CACHE_GROUP_SNAPSHOT_PRELOAD;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.RESTORE_CACHE_GROUP_SNAPSHOT_PREPARE;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.RESTORE_CACHE_GROUP_SNAPSHOT_ROLLBACK;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.RESTORE_CACHE_GROUP_SNAPSHOT_START;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.RESTORE_INCREMENTAL_SNAPSHOT_START;
+import static
org.apache.ignite.internal.util.distributed.DistributedProcess.DistributedProcessType.START_SNAPSHOT;
+import static
org.apache.ignite.testframework.GridTestUtils.assertThrowsAnyCause;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
+
+/** */
+@RunWith(Parameterized.class)
+public class IgniteClusterSnapshotDeleteTest extends AbstractSnapshotSelfTest {
+ /** */
+ private boolean separatedWorkDir;
+
+ /** */
+ @Parameter(2)
+ public boolean incremental = true;
+
+ /** */
+ private @Nullable String cstIdSuffix;
+
+ /** Parameters. */
+ @Parameterized.Parameters(name = "encryption={0}, onlyPrimary={1},
incremental={2}")
+ public static Collection<?> runParams() {
+ Collection<Object[]> res = new ArrayList<>();
+
+ for (boolean incremental : F.asList(false, true)) {
+ for (Object[] src0 : params()) {
+ Object[] res0 = new Object[src0.length + 1];
+ System.arraycopy(src0, 0, res0, 0, src0.length);
+
+ res0[src0.length] = incremental;
+
+ res.add(res0);
+ }
+ }
+
+ return res;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String
igniteInstanceName) throws Exception {
+ var cfg = super.getConfiguration(igniteInstanceName);
+
+ if (separatedWorkDir)
+ cfg.setWorkDirectory(new File(U.defaultWorkDirectory(),
igniteInstanceName).getAbsolutePath());
+
+ if (cstIdSuffix != null)
+ cfg.setConsistentId(cfg.getConsistentId().toString() + '_' +
cstIdSuffix);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void afterTestSnapshot() throws Exception {
+ super.afterTestSnapshot();
+
+ G.allGrids();
+
Review Comment:
`G.allGrids();` is a no-op statement here (return value is ignored, and it
has no side effects). It makes the intent unclear; please remove it, or replace
it with an assertion/logic that uses the returned grids if something was
intended.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -1830,6 +1930,21 @@ public <T> T readFromFile(File smf) throws
IgniteCheckedException, IOException {
* local node will be placed on the first place.
*/
public List<SnapshotMetadata> readSnapshotMetadatas(SnapshotFileTree sft) {
+ return readSnapshotMetadatas(sft, true);
+ }
+
+ /**
+ * Note, there can be snapshots from other nodes.
+ * This method will read all metadata.
+ * Some instances can return {@link SnapshotMetadata#folderName()} and
{@link SnapshotMetadata#consistentId()} that differs from local.
+ *
+ * @param sft Snapshot file tree.
+ * @param failIfCantRead If {@code true}, throws an exeption if cant read
a metadata file.
Review Comment:
Javadoc typos: “exeption” → “exception”, “cant” → “can’t/cannot”.
##########
docs/_docs/snapshots/snapshots.adoc:
##########
@@ -287,6 +287,52 @@ 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
Review Comment:
Typos/grammar in documentation: “pesmissins” → “permissions”, “a not
snapshot” → “a non-snapshot”, “apper” → “appear”, “not subject for” → “not
subject to”, and line 332–333 should be reflowed/clarified. These are public
docs and should be corrected.
##########
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 " +
+ "metadata are prohibited." +
+ U.nl() + U.nl() +
+ "The operation cannot be reverted." +
+ U.nl() + U.nl() +
+ "NOTE: Snapshot data on offline server nodes will remain
untouched.";
+ }
Review Comment:
There are multiple grammar issues in this user-visible confirmation prompt
(e.g., “Deletion in not snapshots Ignite's directories…”). Please rephrase for
clarity (e.g., “Deletion in non-snapshot Ignite directories … is prohibited.”).
This is displayed to end users, so readability matters.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlannerHelper.java:
##########
@@ -134,7 +134,7 @@ public static IgniteRel optimize(SqlNode sqlNode,
IgnitePlanner planner, IgniteL
rel = planner.trimUnusedFields(root.withRel(rel)).rel;
// The following pushed down project can erase top-level hints. We
store them to reassign hints for join nodes.
- // Clear the inherit pathes to consider the hints as not
propogated ones.
+ // Clear the inherit paths to consider the hints as not propogated
ones.
Review Comment:
“propogated” is misspelled; should be “propagated”.
--
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]
