This is an automated email from the ASF dual-hosted git repository.
alex-plekhanov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new c676f7ac326 IGNITE-28870 Restore snapshot temporary files cleanup
logic (#13358)
c676f7ac326 is described below
commit c676f7ac326576774b4647aba26b411a97fc5451
Author: oleg-vlsk <[email protected]>
AuthorDate: Tue Jul 28 19:22:57 2026 +1000
IGNITE-28870 Restore snapshot temporary files cleanup logic (#13358)
---
.../cache/persistence/filename/FileTreeUtils.java | 25 ++--
.../snapshot/IgniteSnapshotManagerSelfTest.java | 134 +++++++++++++++++++++
2 files changed, 148 insertions(+), 11 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/filename/FileTreeUtils.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/filename/FileTreeUtils.java
index 7d1dd7d7f05..7214180479f 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/filename/FileTreeUtils.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/filename/FileTreeUtils.java
@@ -19,6 +19,7 @@ package
org.apache.ignite.internal.processors.cache.persistence.filename;
import java.io.File;
import java.io.IOException;
+import java.nio.file.NoSuchFileException;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@@ -63,10 +64,12 @@ public class FileTreeUtils {
public static void removeTmpSnapshotFiles(SnapshotFileTree sft, boolean
err, IgniteLogger log) {
NodeFileTree tmpFt = sft.tempFileTree();
- removeTmpDir(tmpFt.root(), err, log);
+ U.delete(tmpFt.nodeStorage());
for (File tmpDrStorage : tmpFt.extraStorages().values())
- removeTmpDir(tmpDrStorage.getParentFile(), err, log);
+ U.delete(tmpDrStorage);
+
+ deleteSnapshotTempRoot(tmpFt.root(), err, log);
}
/**
@@ -104,20 +107,20 @@ public class FileTreeUtils {
}
/**
- * @param dir Directory to remove
- * @param err If {@code true} then operation ends with error.
+ * @param root Snapshot temporary root.
+ * @param err {@code True} if snapshot processing finished with an error.
* @param log Logger.
*/
- private static void removeTmpDir(File dir, boolean err, IgniteLogger log) {
- U.delete(dir);
-
- // Delete snapshot directory if no other files exists.
+ private static void deleteSnapshotTempRoot(File root, boolean err,
IgniteLogger log) {
try {
- if (U.fileCount(dir.toPath()) == 0 || err)
- U.delete(dir.toPath());
+ if (err || U.fileCount(root.toPath()) == 0)
+ U.delete(root.toPath());
+ }
+ catch (NoSuchFileException ignored) {
+ // Snapshot temporary root has already been removed.
}
catch (IOException e) {
- log.error("Snapshot directory doesn't exist [snpName=" +
dir.getName() + ", dir=" + dir.getParentFile() + ']');
+ log.error("Failed to clean up snapshot temporary root [dir=" +
root + ']', e);
}
}
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java
index f3edd6df687..353ba0bb5f7 100644
---
a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java
+++
b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.lang.management.ThreadMXBean;
import java.nio.ByteBuffer;
+import java.nio.file.Files;
import java.nio.file.OpenOption;
import java.util.Collections;
import java.util.HashMap;
@@ -59,6 +60,8 @@ import
org.apache.ignite.internal.processors.cache.persistence.file.FilePageStor
import
org.apache.ignite.internal.processors.cache.persistence.file.FilePageStoreManager;
import
org.apache.ignite.internal.processors.cache.persistence.file.FileVersionCheckingFactory;
import
org.apache.ignite.internal.processors.cache.persistence.file.RandomAccessFileIOFactory;
+import
org.apache.ignite.internal.processors.cache.persistence.filename.FileTreeUtils;
+import
org.apache.ignite.internal.processors.cache.persistence.filename.NodeFileTree;
import
org.apache.ignite.internal.processors.cache.persistence.filename.SnapshotFileTree;
import
org.apache.ignite.internal.processors.cache.persistence.partstate.GroupPartitionId;
import org.apache.ignite.internal.util.GridCloseableIteratorAdapter;
@@ -94,6 +97,9 @@ public class IgniteSnapshotManagerSelfTest extends
AbstractSnapshotSelfTest {
/** Number of threads being used to perform snapshot operation. */
private Integer snapshotThreadPoolSize;
+ /** Extra storage root configured for the current test. */
+ private File extraStorageRoot;
+
/** {@inheritDoc} */
@Override protected IgniteConfiguration getConfiguration(String
igniteInstanceName) throws Exception {
IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
@@ -102,6 +108,9 @@ public class IgniteSnapshotManagerSelfTest extends
AbstractSnapshotSelfTest {
// listener registration and calling snpFutTask.start().
cfg.getDataStorageConfiguration().setCheckpointFrequency(TimeUnit.DAYS.toMillis(365));
+ if (extraStorageRoot != null)
+
cfg.getDataStorageConfiguration().setExtraStoragePaths(extraStorageRoot.getAbsolutePath());
+
if (listenLog != null)
cfg.setGridLogger(listenLog);
@@ -111,6 +120,18 @@ public class IgniteSnapshotManagerSelfTest extends
AbstractSnapshotSelfTest {
return cfg;
}
+ /** {@inheritDoc} */
+ @Override public void afterTestSnapshot() throws Exception {
+ try {
+ super.afterTestSnapshot();
+ }
+ finally {
+ U.delete(extraStorageRoot);
+
+ extraStorageRoot = null;
+ }
+ }
+
/**
* Test that all partitions are copied successfully even after multiple
checkpoints occur during
* the long copy of cache partition files.
@@ -647,6 +668,119 @@ public class IgniteSnapshotManagerSelfTest extends
AbstractSnapshotSelfTest {
assertFalse(noMatchParams.check());
}
+ /**
+ * Tests that snapshot temporary root is not removed on successful cleanup
if it still contains files after
+ * node-specific storage has been removed.
+ */
+ @Test
+ public void testSnapshotTmpRootIsNotRemovedIfNotEmptyOnSuccess() throws
Exception {
+ IgniteEx ignite = startGridWithCache(dfltCacheCfg, CACHE_KEYS_RANGE);
+
+ SnapshotFileTree sft = snapshotFileTree(ignite, SNAPSHOT_NAME);
+
+ NodeFileTree tmpFt = sft.tempFileTree();
+
+ File root = tmpFt.root();
+ File nodeStorage = tmpFt.nodeStorage();
+
+ assertTrue(nodeStorage.mkdirs());
+
+ File extraFile = new File(root, "unexpected-tmp-file");
+
+ assertTrue(extraFile.createNewFile());
+
+ try {
+ FileTreeUtils.removeTmpSnapshotFiles(sft, false, log);
+
+ assertFalse("Node-specific temporary storage must be removed: " +
nodeStorage, nodeStorage.exists());
+
+ assertTrue("Snapshot temporary root must not be removed if it is
not empty: " + root, root.exists());
+
+ assertTrue("Unexpected temporary file must not be removed on
successful cleanup: " + extraFile,
+ extraFile.exists());
+ }
+ finally {
+ U.delete(root);
+ }
+ }
+
+ /**
+ * Tests that snapshot temporary root is removed on cleanup after an error
even if it still contains files after
+ * node-specific storage has been removed.
+ */
+ @Test
+ public void testSnapshotTmpRootIsRemovedIfNotEmptyOnError() throws
Exception {
+ IgniteEx ignite = startGridWithCache(dfltCacheCfg, CACHE_KEYS_RANGE);
+
+ SnapshotFileTree sft = snapshotFileTree(ignite, SNAPSHOT_NAME);
+
+ NodeFileTree tmpFt = sft.tempFileTree();
+
+ File root = tmpFt.root();
+ File nodeStorage = tmpFt.nodeStorage();
+
+ assertTrue(nodeStorage.mkdirs());
+
+ File extraFile = new File(root, "unexpected-tmp-file");
+
+ assertTrue(extraFile.createNewFile());
+
+ FileTreeUtils.removeTmpSnapshotFiles(sft, true, log);
+
+ assertFalse("Node-specific temporary storage must be removed: " +
nodeStorage, nodeStorage.exists());
+
+ assertFalse("Snapshot temporary root must be removed on error: " +
root, root.exists());
+ }
+
+ /**
+ * Tests cleanup of temporary snapshot files located in an extra storage.
+ * <p>
+ * Unlike the default storage, where the node-specific storage is nested
inside the temporary root of the
+ * current snapshot, an extra storage entry already points to the
snapshot-specific temporary directory:
+ *
+ * <pre>
+ * <extra-storage>/snp/ - common snapshot
temporary root
+ * <extra-storage>/snp/<snapshot-name> -
snapshot-specific extra storage
+ * </pre>
+ *
+ * Therefore, cleanup must remove the snapshot-specific extra storage
directory but preserve its common
+ * temporary root.
+ */
+ @Test
+ public void testSnapshotTmpExtraStorageCleanupPreservesTempRoot() throws
Exception {
+ IgniteEx ignite = startGridWithExtraStorage();
+
+ SnapshotFileTree sft = snapshotFileTree(ignite, SNAPSHOT_NAME);
+
+ NodeFileTree tmpFt = sft.tempFileTree();
+
+ assertEquals(1, tmpFt.extraStorages().size());
+
+ File tmpExtraStorage =
tmpFt.extraStorages().values().iterator().next();
+ File tmpExtraRoot = tmpExtraStorage.getParentFile();
+
+ assertTrue(tmpExtraStorage.mkdirs() || tmpExtraStorage.isDirectory());
+
+ FileTreeUtils.removeTmpSnapshotFiles(sft, false, log);
+
+ assertFalse("Snapshot-specific temporary extra storage must be
removed: " + tmpExtraStorage,
+ tmpExtraStorage.exists());
+
+ assertTrue("Common snapshot temporary root must be preserved: " +
tmpExtraRoot,
+ tmpExtraRoot.isDirectory());
+ }
+
+ /** */
+ private IgniteEx startGridWithExtraStorage() throws Exception {
+ extraStorageRoot =
Files.createTempDirectory("ignite-snapshot-extra-storage-").toFile();
+
+ CacheConfiguration<Integer, Object> ccfg = new
CacheConfiguration<>(dfltCacheCfg);
+
+ ccfg.setStoragePaths(extraStorageRoot.getAbsolutePath());
+
+ return startGridWithCache(ccfg, CACHE_KEYS_RANGE);
+ }
+
/**
* @param ignite Ignite instance to set factory.
* @param factory New factory to use.
