zstan commented on code in PR #13577: URL: https://github.com/apache/ignite/pull/13577#discussion_r4130300004
########## 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 { Review Comment: passed for 6 min for me, seems it is a huge time and need additional suite if you think that all cases are acceptable, as for me - this test is very hard to understand for now. ########## modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotDeleteTest.java: ########## @@ -0,0 +1,1019 @@ +/* + * 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.IOException; +import java.io.RandomAccessFile; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Random; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteException; +import org.apache.ignite.IgniteIllegalStateException; +import org.apache.ignite.configuration.DataStorageConfiguration; +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.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.T2; +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.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; + +/** */ +public class IgniteClusterSnapshotDeleteTest extends AbstractSnapshotSelfTest { + /** */ + private static final int INC_CACHE_KEYS_RANGE = CACHE_KEYS_RANGE + CACHE_KEYS_RANGE / 4; + + /** Extra storage path. */ + private static final String EXT_STORAGE_PATH = "extStorage"; + + /** */ + private static boolean CASE_INSENSETIVE_FS; + + /** */ + private static boolean POSIX_PERMISSIONS; + + /** */ + private boolean separatedWorkDir; + + /** */ + private boolean extraStorages; + + /** */ + @Parameter(2) + public boolean incremental = true; + + /** */ + private @Nullable String cstIdSuffix; + + /** Sets the extra snapshot storage to {@link DataStorageConfiguration#setExtraSnapshotPaths(String...)}. */ + private boolean lowerCasedSnpName; Review Comment: I missed this stuff, despite of FS sensitivity all tests need to pass, thus i don\`t understand this approach ########## docs/_docs/snapshots/snapshots.adoc: ########## @@ -287,6 +287,50 @@ 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 "C:\tmp\ignite\snapshots" folder. +control.bat --snapshot delete snapshot_09062021 --src C:\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 permissions 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 Review Comment: ```suggestion * Before deletion, no validation is performed on the snapshot other than locating and reading its metadata. If the metadata isn't found ``` ########## modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotDeleteTest.java: ########## @@ -0,0 +1,1019 @@ +/* + * 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.IOException; +import java.io.RandomAccessFile; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Random; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteException; +import org.apache.ignite.IgniteIllegalStateException; +import org.apache.ignite.configuration.DataStorageConfiguration; +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.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.T2; +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.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; + +/** */ +public class IgniteClusterSnapshotDeleteTest extends AbstractSnapshotSelfTest { + /** */ + private static final int INC_CACHE_KEYS_RANGE = CACHE_KEYS_RANGE + CACHE_KEYS_RANGE / 4; + + /** Extra storage path. */ + private static final String EXT_STORAGE_PATH = "extStorage"; + + /** */ + private static boolean CASE_INSENSETIVE_FS; + + /** */ + private static boolean POSIX_PERMISSIONS; + + /** */ + private boolean separatedWorkDir; + + /** */ + private boolean extraStorages; + + /** */ + @Parameter(2) + public boolean incremental = true; + + /** */ + private @Nullable String cstIdSuffix; + + /** Sets the extra snapshot storage to {@link DataStorageConfiguration#setExtraSnapshotPaths(String...)}. */ + private boolean lowerCasedSnpName; + + /** */ + private @Nullable String[] extStoragePaths; + + /** Parameters. */ + @Parameterized.Parameters(name = "encryption={0}, onlyPrimary={1}, incremental={2}") + public static Collection<?> runParams() { + /** Use {@link #incremental} only. */ + return F.asList( + new Object[] {false, false, false}, + new Object[] {false, false, true} + ); + } + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + var cfg = super.getConfiguration(igniteInstanceName); + + String workDir = separatedWorkDir + ? new File(U.defaultWorkDirectory(), igniteInstanceName).getAbsolutePath() + : U.defaultWorkDirectory(); + + cfg.setWorkDirectory(workDir); + + if (cstIdSuffix != null) + cfg.setConsistentId(cfg.getConsistentId().toString() + '_' + cstIdSuffix); + + if (extraStorages) { + cfg.getDataStorageConfiguration().setExtraStoragePaths( + workDir + File.separator, + workDir + File.separator + EXT_STORAGE_PATH + ); + + extStoragePaths = cfg.getDataStorageConfiguration().getExtraStoragePaths(); + + cfg.getDataStorageConfiguration().setExtraSnapshotPaths("", EXT_STORAGE_PATH); + } + + return cfg; + } + + /** {@inheritDoc} */ + @Override public void afterTestSnapshot() throws Exception { + super.afterTestSnapshot(); + + cleanPersistenceDir(); + } + + /** {@inheritDoc} */ + @Override public void beforeTestSnapshot() throws Exception { + super.beforeTestSnapshot(); + + /** Handy if test running is interrupted and {@link #afterTestSnapshot()} isn't invoked. */ + cleanPersistenceDir(); + } + + /** {@inheritDoc} */ + @Override protected void beforeTestsStarted() throws Exception { + super.beforeTestsStarted(); + + File workDir = new File(U.defaultWorkDirectory()); + + assertTrue(workDir.exists()); + + CASE_INSENSETIVE_FS = new File(workDir.getAbsolutePath().toLowerCase()).exists() && + new File(workDir.getAbsolutePath().toUpperCase()).exists(); + + Path workPath = workDir.toPath(); + + try { + Files.getPosixFilePermissions(workPath); + + POSIX_PERMISSIONS = true; + } + catch (IOException ignored) { + // No-op. + } + } + + /** */ + @Test + public void testDeniedPermissions() throws Exception { + assumeTrue(POSIX_PERMISSIONS); + + // Doesn't matter here. + assumeFalse(incremental); + + separatedWorkDir = true; + + AtomicReference<Set<PosixFilePermission>> prevPerms = new AtomicReference<>(); + AtomicReference<Path> pathRef = new AtomicReference<>(); + + pluginProvider = new AbstractTestPluginProvider() { + @Override public String name() { + return "TestSnpMgrProvider"; + } + + @Override public <T> T createComponent(PluginContext ctx, Class<T> cls) { + if (IgniteSnapshotManager.class.isAssignableFrom(cls)) { + return (T)new IgniteSnapshotManager(((IgniteEx)ctx.grid()).context()) { + @Override public T2<Boolean, Boolean> deleteLocalSnapshot(SnapshotFileTree sft) { + if (ctx.localNode().id().equals(grid(1).localNode().id())) { + Path path = sft.root().toPath(); + + pathRef.set(path); + + try { + prevPerms.set(Files.getPosixFilePermissions(path)); + + // Denies writing (deletion). + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("r-xr-x---")); + } + catch (Exception e) { + throw new IgniteException("Unable to set the posix permissions.", e); + } + } + + return super.deleteLocalSnapshot(sft); + } + }; + } + + return super.createComponent(ctx, cls); + } + }; + + startGridsWithCache(3, CACHE_KEYS_RANGE, i -> i, dfltCacheCfg); + + snp(grid(0)).createSnapshot(SNAPSHOT_NAME).get(getTestTimeout()); + + try { + SnapshotDeleteProcessResult res = snp(grid(0)).deleteSnapshot(SNAPSHOT_NAME, null).get(getTestTimeout()); + + assertEquals(1, res.uncompletedNodes().size()); + assertEquals(2, res.completedNodes().size()); + } + finally { + if (pathRef.get() != null && pathRef.get() != null) + Files.setPosixFilePermissions(pathRef.get(), prevPerms.get()); + } + } + + /** */ + @Test + public void testExtraStoragesDeleted() throws Exception { + extraStorages = true; + + dfltCacheCfg = null; + + startGridsMultiThreaded(3); + + assertFalse(F.isEmpty(extStoragePaths)); + + assertTrue(grid(0).cache(DEFAULT_CACHE_NAME) == null); + + dfltCacheCfg = defaultCacheConfiguration(); + + // Works only with a shared work directory. + dfltCacheCfg.setStoragePaths(extStoragePaths); + + grid(0).createCache(dfltCacheCfg); + awaitPartitionMapExchange(); + + // Fills the cache. + startGridsWithCache(0, CACHE_KEYS_RANGE, i -> i, dfltCacheCfg); + + snp(grid(0)).createSnapshot(SNAPSHOT_NAME, null, false, onlyPrimary).get(getTestTimeout()); Review Comment: I believe it need to work also for absolute path\`s but it doesn\`t, I mean : ``` snp(grid(0)).createSnapshot(SNAPSHOT_NAME, "/tmp/sNp_path", false, onlyPrimary).get(getTestTimeout()); snp(grid(1)).deleteSnapshot(SNAPSHOT_NAME, "/tmp/sNp_path").get(getTestTimeout()); ``` after this operations "/tmp/sNp_path" is not empty ########## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotDeleteProcess.java: ########## @@ -0,0 +1,389 @@ +/* + * 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.IOException; +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 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.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.T2; +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. "; + + /** */ + public static final String OP_REJECT_FEATURE_MSG = OP_REJECT_MSG + "The snapshot deletion feature isn't activated yet."; + + /** 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) { + 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_FEATURE_MSG)); + + return new IgniteFutureImpl<>(clusterOpFut); + } + + UUID reqId = UUID.randomUUID(); + + clusterOpFut.listen(fut -> clusterOpFuts.remove(reqId)); + + try { + 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 the same name is being created [req=" + req + ']')); + } + + if (snpMgr.isRestoring(req.snpName)) { + return new GridFinishedFuture<>(new IgniteIllegalStateException(OP_REJECT_MSG + + "Snapshot with the same name is being restored [req=" + req + ']')); + } + + if (snpMgr.isSnapshotChecking(req.snpName)) { + return new GridFinishedFuture<>(new IgniteIllegalStateException(OP_REJECT_MSG + + "Snapshot with the same name is being checked [req=" + req + ']')); + } + + try { + File path = resolvePath(req.snpPath); + + req.resolvedPath = path; + + 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 snapshot 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 { + // Read file tree of the snapshot. + var byMetaSft = new SnapshotFileTree(kctx, req.snpName, path0.getAbsolutePath(), meta.folderName(), + meta.consId); + + T2<Boolean, Boolean> deleted = snpMgr.deleteLocalSnapshot(byMetaSft); + + SnapshotDeleteResponse.DeleteStatus status; + + // If found. + if (deleted.get2()) { + if (deleted.get1() && log.isInfoEnabled()) + log.info("Snapshot successfully deleted [req=" + req + ']'); + else if (!deleted.get1()) + log.warning("Snapshot deleted not completely [req=" + req + ']'); + + status = deleted.get1() + ? 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) throws IOException { + File res = kctx.pdsFolderResolver().fileTree().snapshotsRoot(); + + if (path != null) { + File reqPath = new File(path); + + res = reqPath.isAbsolute() ? reqPath : new File(res, path); Review Comment: I think it need to be fixed under this scope (not this issue) add mention test and disable it under appropriate issue? ########## modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java: ########## @@ -0,0 +1,132 @@ +/* + * 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 the snapshot and all its incremental snapshots from all 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 = "Snapshot not found on available 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); + } + } + + /** */ + 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); Review Comment: the same as above ########## modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotDeleteTest.java: ########## @@ -0,0 +1,1019 @@ +/* + * 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.IOException; +import java.io.RandomAccessFile; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Random; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteException; +import org.apache.ignite.IgniteIllegalStateException; +import org.apache.ignite.configuration.DataStorageConfiguration; +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.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.T2; +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.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; + +/** */ +public class IgniteClusterSnapshotDeleteTest extends AbstractSnapshotSelfTest { + /** */ + private static final int INC_CACHE_KEYS_RANGE = CACHE_KEYS_RANGE + CACHE_KEYS_RANGE / 4; + + /** Extra storage path. */ + private static final String EXT_STORAGE_PATH = "extStorage"; + + /** */ + private static boolean CASE_INSENSETIVE_FS; + + /** */ + private static boolean POSIX_PERMISSIONS; + + /** */ + private boolean separatedWorkDir; + + /** */ + private boolean extraStorages; + + /** */ + @Parameter(2) + public boolean incremental = true; + + /** */ + private @Nullable String cstIdSuffix; + + /** Sets the extra snapshot storage to {@link DataStorageConfiguration#setExtraSnapshotPaths(String...)}. */ + private boolean lowerCasedSnpName; + + /** */ + private @Nullable String[] extStoragePaths; + + /** Parameters. */ + @Parameterized.Parameters(name = "encryption={0}, onlyPrimary={1}, incremental={2}") + public static Collection<?> runParams() { + /** Use {@link #incremental} only. */ + return F.asList( + new Object[] {false, false, false}, + new Object[] {false, false, true} + ); + } + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + var cfg = super.getConfiguration(igniteInstanceName); + + String workDir = separatedWorkDir + ? new File(U.defaultWorkDirectory(), igniteInstanceName).getAbsolutePath() + : U.defaultWorkDirectory(); + + cfg.setWorkDirectory(workDir); + + if (cstIdSuffix != null) + cfg.setConsistentId(cfg.getConsistentId().toString() + '_' + cstIdSuffix); + + if (extraStorages) { + cfg.getDataStorageConfiguration().setExtraStoragePaths( + workDir + File.separator, + workDir + File.separator + EXT_STORAGE_PATH + ); + + extStoragePaths = cfg.getDataStorageConfiguration().getExtraStoragePaths(); + + cfg.getDataStorageConfiguration().setExtraSnapshotPaths("", EXT_STORAGE_PATH); Review Comment: I found that snapshot creation duplicates data in such a case, i mean : Also i found a strange structure for such a case : ``` /ignite/work/snapshots/testSnapshot/db /ignite/work/extStorage/snapshots/testSnapshot/db ``` is it a bug ? probably some data duplication ? If i\`am right - seems an issue need here, wdyt ? ########## modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotSelfTest.java: ########## @@ -283,6 +297,25 @@ public void afterTestSnapshot() throws Exception { cleanPersistenceDir(); } + /** {@inheritDoc} */ + @Override protected void cleanPersistenceDir() throws Exception { + super.cleanPersistenceDir(); + + if (!fullCleanPersistentDir()) Review Comment: full clean \ not full clean, what does it mean ??? can you rename it somehow like: "remove|clear|SnapstotDirectory" ########## modules/core/src/main/java/org/apache/ignite/internal/management/snapshot/SnapshotDeleteCommand.java: ########## @@ -0,0 +1,132 @@ +/* + * 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 the snapshot and all its incremental snapshots from all 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 = "Snapshot not found on available 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); + } + } + + /** */ + private static String nodeIdPairsStrLst(Map<UUID, String> uuids) { + return "[cnt=" + uuids.size() + "]: " + uuids.entrySet().stream() Review Comment: I already told that such kind of message is not acceptable\common for Ignite project (or fix me if i\`m wrong). Sounds weird : "Snapshot removal is completed on **[cnt=3]**:" -- 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]
