From f2f6befeb31986153a6f63ea3fed507b01e8a5c4 Mon Sep 17 00:00:00 2001 From: "Valuyskiy.O.Y" Date: Sat, 11 Jul 2026 15:29:24 +1000 Subject: [PATCH 1/3] IGNITE-28870 Initial commit --- .../persistence/filename/FileTreeUtils.java | 26 +++++++----- .../IgniteSnapshotManagerSelfTest.java | 30 ++++++++++--- .../snapshot/SnapshotTmpDirCleanupTest.java | 42 +++++++++++++++++++ 3 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java 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 7d1dd7d7f05d3..ea105c7dd198b 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 @@ 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,10 @@ public static void createCacheStorages(NodeFileTree ft, IgniteLogger log) throws public static void removeTmpSnapshotFiles(SnapshotFileTree sft, boolean err, IgniteLogger log) { NodeFileTree tmpFt = sft.tempFileTree(); - removeTmpDir(tmpFt.root(), err, log); + removeTmpDir(tmpFt.nodeStorage(), tmpFt.root(), err, log); for (File tmpDrStorage : tmpFt.extraStorages().values()) - removeTmpDir(tmpDrStorage.getParentFile(), err, log); + removeTmpDir(tmpDrStorage, tmpDrStorage.getParentFile(), err, log); } /** @@ -104,20 +105,25 @@ public static Set nodeStorages(DataStorageConfiguration dsCfg) throws Ig } /** - * @param dir Directory to remove - * @param err If {@code true} then operation ends with error. + * @param nodeStorage Node-specific temporary snapshot storage directory. + * @param root Snapshot temporary root directory. + * @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); + private static void removeTmpDir(File nodeStorage, File root, boolean err, IgniteLogger log) { + U.delete(nodeStorage); - // Delete snapshot directory if no other files exists. + // Delete snapshot temporary root if no other files exist or snapshot cleanup is performed after an error. 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) { + // Temporary snapshot 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 directory " + + "[snpName=" + root.getName() + ", 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 f3edd6df687da..ed147e6ec3d3e 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 @@ -94,6 +94,9 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { /** Number of threads being used to perform snapshot operation. */ private Integer snapshotThreadPoolSize; + /** Log listener for snapshot temporary directory cleanup routine. */ + private LogListener tmpDirCleanupLsnr; + /** {@inheritDoc} */ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); @@ -102,8 +105,9 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { // listener registration and calling snpFutTask.start(). cfg.getDataStorageConfiguration().setCheckpointFrequency(TimeUnit.DAYS.toMillis(365)); - if (listenLog != null) - cfg.setGridLogger(listenLog); + assertNotNull(listenLog); + + cfg.setGridLogger(listenLog); if (nonNull(snapshotThreadPoolSize)) cfg.setSnapshotThreadPoolSize(snapshotThreadPoolSize); @@ -111,6 +115,24 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { return cfg; } + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + listenLog = new ListeningTestLogger(log); + + tmpDirCleanupLsnr = LogListener.matches("Failed to clean up snapshot temporary directory [snpName=").build(); + + listenLog.registerListener(tmpDirCleanupLsnr); + } + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + assertFalse("Snapshot temporary directory cleanup error was logged", tmpDirCleanupLsnr.check()); + + super.afterTest(); + } + /** * Test that all partitions are copied successfully even after multiple checkpoints occur during * the long copy of cache partition files. @@ -525,8 +547,6 @@ public void testSnapshotIteratorLargeRows() throws Exception { public void testSnapshotAlwaysStartsNewCheckpoint() throws Exception { long testTimeout = 30_000; - listenLog = new ListeningTestLogger(log); - LogListener lsnr = LogListener.matches("Snapshot operation is scheduled on local node").times(1).build(); listenLog.registerListener(lsnr); @@ -607,8 +627,6 @@ public void testSnapshotThreadPoolSizeUsage() throws Exception { public void testFullSnapshotCreationLog() throws Exception { assumeFalse("https://issues.apache.org/jira/browse/IGNITE-17819", encryption); - listenLog = new ListeningTestLogger(log); - final int entriesCnt = 4; LogListener matchStart = LogListener.matches("Cluster-wide snapshot operation started: ").times(entriesCnt).build(); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java new file mode 100644 index 0000000000000..845d141229e39 --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java @@ -0,0 +1,42 @@ +package org.apache.ignite.internal.processors.cache.persistence.snapshot; + +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.apache.ignite.internal.util.typedef.internal.U; +import org.apache.ignite.testframework.ListeningTestLogger; +import org.apache.ignite.testframework.LogListener; +import org.junit.Test; + +/** */ +public class SnapshotTmpDirCleanupTest extends AbstractSnapshotSelfTest { + /** */ + private final ListeningTestLogger listeningLog = new ListeningTestLogger(log); + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + if (listeningLog != null) + cfg.setGridLogger(listeningLog); + + return cfg; + } + + /** */ + @Test + public void testClusterSnapshotCheck() throws Exception { + String snpDir = U.defaultWorkDirectory() + "/db/snapshot_" + getClass().getSimpleName() + "0/snp"; + + String msg = String.format("Snapshot directory doesn't exist [snpName=%s, dir=%s]", SNAPSHOT_NAME, snpDir); + + IgniteEx ignite = startGridsWithCache(1, dfltCacheCfg, CACHE_KEYS_RANGE); + + LogListener lsnr = LogListener.matches(msg).build(); + + listeningLog.registerListener(lsnr); + + createAndCheckSnapshot(ignite, SNAPSHOT_NAME); + + assertFalse(lsnr.check()); + } +} From 9569537eda24529c5cc790370caa14c8bb2f4f95 Mon Sep 17 00:00:00 2001 From: "Valuyskiy.O.Y" Date: Sun, 12 Jul 2026 07:55:10 +1000 Subject: [PATCH 2/3] IGNITE-28870 Restore snapshot temporary files cleanup logic --- .../persistence/filename/FileTreeUtils.java | 6 +- .../IgniteSnapshotManagerSelfTest.java | 96 ++++++++++++++----- .../snapshot/SnapshotTmpDirCleanupTest.java | 42 -------- 3 files changed, 75 insertions(+), 69 deletions(-) delete mode 100644 modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java 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 ea105c7dd198b..1685ac48a9556 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 @@ -105,13 +105,13 @@ public static Set nodeStorages(DataStorageConfiguration dsCfg) throws Ig } /** - * @param nodeStorage Node-specific temporary snapshot storage directory. + * @param storage Temporary snapshot storage directory. * @param root Snapshot temporary root directory. * @param err {@code True} if snapshot processing finished with an error. * @param log Logger. */ - private static void removeTmpDir(File nodeStorage, File root, boolean err, IgniteLogger log) { - U.delete(nodeStorage); + private static void removeTmpDir(File storage, File root, boolean err, IgniteLogger log) { + U.delete(storage); // Delete snapshot temporary root if no other files exist or snapshot cleanup is performed after an error. try { 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 ed147e6ec3d3e..265942fd7fc48 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 @@ -59,6 +59,8 @@ 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,9 +96,6 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { /** Number of threads being used to perform snapshot operation. */ private Integer snapshotThreadPoolSize; - /** Log listener for snapshot temporary directory cleanup routine. */ - private LogListener tmpDirCleanupLsnr; - /** {@inheritDoc} */ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); @@ -105,9 +104,8 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { // listener registration and calling snpFutTask.start(). cfg.getDataStorageConfiguration().setCheckpointFrequency(TimeUnit.DAYS.toMillis(365)); - assertNotNull(listenLog); - - cfg.setGridLogger(listenLog); + if (listenLog != null) + cfg.setGridLogger(listenLog); if (nonNull(snapshotThreadPoolSize)) cfg.setSnapshotThreadPoolSize(snapshotThreadPoolSize); @@ -115,24 +113,6 @@ public class IgniteSnapshotManagerSelfTest extends AbstractSnapshotSelfTest { return cfg; } - /** {@inheritDoc} */ - @Override protected void beforeTest() throws Exception { - super.beforeTest(); - - listenLog = new ListeningTestLogger(log); - - tmpDirCleanupLsnr = LogListener.matches("Failed to clean up snapshot temporary directory [snpName=").build(); - - listenLog.registerListener(tmpDirCleanupLsnr); - } - - /** {@inheritDoc} */ - @Override protected void afterTest() throws Exception { - assertFalse("Snapshot temporary directory cleanup error was logged", tmpDirCleanupLsnr.check()); - - super.afterTest(); - } - /** * Test that all partitions are copied successfully even after multiple checkpoints occur during * the long copy of cache partition files. @@ -547,6 +527,8 @@ public void testSnapshotIteratorLargeRows() throws Exception { public void testSnapshotAlwaysStartsNewCheckpoint() throws Exception { long testTimeout = 30_000; + listenLog = new ListeningTestLogger(log); + LogListener lsnr = LogListener.matches("Snapshot operation is scheduled on local node").times(1).build(); listenLog.registerListener(lsnr); @@ -627,6 +609,8 @@ public void testSnapshotThreadPoolSizeUsage() throws Exception { public void testFullSnapshotCreationLog() throws Exception { assumeFalse("https://issues.apache.org/jira/browse/IGNITE-17819", encryption); + listenLog = new ListeningTestLogger(log); + final int entriesCnt = 4; LogListener matchStart = LogListener.matches("Cluster-wide snapshot operation started: ").times(entriesCnt).build(); @@ -665,6 +649,70 @@ public void testFullSnapshotCreationLog() throws Exception { 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()); + } + /** * @param ignite Ignite instance to set factory. * @param factory New factory to use. diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java deleted file mode 100644 index 845d141229e39..0000000000000 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotTmpDirCleanupTest.java +++ /dev/null @@ -1,42 +0,0 @@ -package org.apache.ignite.internal.processors.cache.persistence.snapshot; - -import org.apache.ignite.configuration.IgniteConfiguration; -import org.apache.ignite.internal.IgniteEx; -import org.apache.ignite.internal.util.typedef.internal.U; -import org.apache.ignite.testframework.ListeningTestLogger; -import org.apache.ignite.testframework.LogListener; -import org.junit.Test; - -/** */ -public class SnapshotTmpDirCleanupTest extends AbstractSnapshotSelfTest { - /** */ - private final ListeningTestLogger listeningLog = new ListeningTestLogger(log); - - /** {@inheritDoc} */ - @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { - IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); - - if (listeningLog != null) - cfg.setGridLogger(listeningLog); - - return cfg; - } - - /** */ - @Test - public void testClusterSnapshotCheck() throws Exception { - String snpDir = U.defaultWorkDirectory() + "/db/snapshot_" + getClass().getSimpleName() + "0/snp"; - - String msg = String.format("Snapshot directory doesn't exist [snpName=%s, dir=%s]", SNAPSHOT_NAME, snpDir); - - IgniteEx ignite = startGridsWithCache(1, dfltCacheCfg, CACHE_KEYS_RANGE); - - LogListener lsnr = LogListener.matches(msg).build(); - - listeningLog.registerListener(lsnr); - - createAndCheckSnapshot(ignite, SNAPSHOT_NAME); - - assertFalse(lsnr.check()); - } -} From 0cf08764bf19e24c8a4298e7d9cfdee797bea50d Mon Sep 17 00:00:00 2001 From: "Valuyskiy.O.Y" Date: Mon, 20 Jul 2026 17:22:37 +1000 Subject: [PATCH 3/3] IGNITE-28870 Replace FileTreeUtils#removeTmpDir with FileTreeUtils#deleteSnapshotTempRoot --- .../persistence/filename/FileTreeUtils.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 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 1685ac48a9556..e6406cbb560d4 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 @@ -64,10 +64,15 @@ public static void createCacheStorages(NodeFileTree ft, IgniteLogger log) throws public static void removeTmpSnapshotFiles(SnapshotFileTree sft, boolean err, IgniteLogger log) { NodeFileTree tmpFt = sft.tempFileTree(); - removeTmpDir(tmpFt.nodeStorage(), tmpFt.root(), err, log); + U.delete(tmpFt.nodeStorage()); - for (File tmpDrStorage : tmpFt.extraStorages().values()) - removeTmpDir(tmpDrStorage, tmpDrStorage.getParentFile(), err, log); + for (File tmpDrStorage : tmpFt.extraStorages().values()) { + U.delete(tmpDrStorage); + + deleteSnapshotTempRoot(tmpDrStorage.getParentFile(), err, log); + } + + deleteSnapshotTempRoot(tmpFt.root(), err, log); } /** @@ -105,25 +110,20 @@ public static Set nodeStorages(DataStorageConfiguration dsCfg) throws Ig } /** - * @param storage Temporary snapshot storage directory. - * @param root Snapshot temporary root directory. + * @param root Snapshot temporary root. * @param err {@code True} if snapshot processing finished with an error. * @param log Logger. */ - private static void removeTmpDir(File storage, File root, boolean err, IgniteLogger log) { - U.delete(storage); - - // Delete snapshot temporary root if no other files exist or snapshot cleanup is performed after an error. + private static void deleteSnapshotTempRoot(File root, boolean err, IgniteLogger log) { try { if (err || U.fileCount(root.toPath()) == 0) U.delete(root.toPath()); } catch (NoSuchFileException ignored) { - // Temporary snapshot root has already been removed. + // Snapshot temporary root has already been removed. } catch (IOException e) { - log.error("Failed to clean up snapshot temporary directory " + - "[snpName=" + root.getName() + ", dir=" + root + ']', e); + log.error("Failed to clean up snapshot temporary root [dir=" + root + ']', e); } }