From 4080de0a02865f2aa8ec315a92c8a1985cb05fa9 Mon Sep 17 00:00:00 2001 From: didar shayarov Date: Mon, 31 Aug 2026 00:54:10 +0300 Subject: [PATCH 1/4] IGNITE-13989 Destroy of persisted cache doesn't remove cache folder --- .../cache/GridLocalConfigManager.java | 23 ++ .../IgnitePdsDestroyRemovesCacheDirTest.java | 220 ++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java index 324849ec7af79..a7eab04ab60f9 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java @@ -24,7 +24,9 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.io.OutputStream; +import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayList; @@ -412,6 +414,27 @@ public void removeCacheGroupConfigurationData(CacheGroupContext ctx) throws Igni throw new IgniteCheckedException("Failed to delete cache configurations of group: " + ctx); } } + + // Remove leftover (now empty) cache storage directories so that PDS is not cluttered with empty + // directories after cache group destroy, see IGNITE-13989. + for (File dir : ft.cacheStorages(ctx.config())) { + if (!dir.exists()) + continue; + + try { + Files.delete(dir.toPath()); + } + catch (NoSuchFileException ignored) { + // Directory has already been removed by someone else. + } + catch (DirectoryNotEmptyException e) { + log.warning("Cache storage directory is not empty and cannot be removed: " + dir.getAbsolutePath()); + } + catch (IOException e) { + log.warning("Failed to remove cache storage directory [dir=" + dir.getAbsolutePath() + + ", reason=" + e.getMessage() + ']'); + } + } } /** diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java new file mode 100644 index 0000000000000..0d75b5cae10db --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java @@ -0,0 +1,220 @@ +/* + * 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; + +import java.io.File; +import java.util.Arrays; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.cluster.ClusterState; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.DataRegionConfiguration; +import org.apache.ignite.configuration.DataStorageConfiguration; +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.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +/** + * Tests that {@link IgniteCache#destroy()} removes the (empty) cache storage directories from the persistent storage, + * see IGNITE-13989. + */ +public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest { + /** Cache name for the shared cache group tests. */ + private static final String CACHE_1 = "cache-1"; + + /** Cache name for the shared cache group tests. */ + private static final String CACHE_2 = "cache-2"; + + /** Cache group name. */ + private static final String GROUP_NAME = "grp"; + + /** Additional storage path for multi-storage test. */ + private File extraStorage; + + /** Separate index path for index storage test. */ + private File indexPath; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setConsistentId(igniteInstanceName); + + DataStorageConfiguration dsCfg = new DataStorageConfiguration() + .setDefaultDataRegionConfiguration( + new DataRegionConfiguration().setPersistenceEnabled(true) + ); + + if (extraStorage != null) + dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath()); + + if (indexPath != null) { + // The index storage must be one of the DataStorageConfiguration storage paths, otherwise cache + // start validation fails. + if (extraStorage != null) + dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath(), indexPath.getAbsolutePath()); + else + dsCfg.setExtraStoragePaths(indexPath.getAbsolutePath()); + } + + cfg.setDataStorageConfiguration(dsCfg); + + return cfg; + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + stopAllGrids(); + + cleanPersistenceDir(); + + extraStorage = null; + indexPath = null; + } + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + stopAllGrids(); + + cleanPersistenceDir(); + + if (extraStorage != null) + U.delete(extraStorage); + + if (indexPath != null) + U.delete(indexPath); + + super.afterTest(); + } + + /** + * @param ignite Node. + * @param ccfg Cache configuration. + * @return Storage directories of the cache group. + */ + private File[] cacheStorageDirs(IgniteEx ignite, CacheConfiguration ccfg) { + return ignite.context().pdsFolderResolver().fileTree().cacheStorages(ccfg); + } + + /** + * @param ignite Node. + * @param ccfg Cache configuration. + */ + private void assertStorageDirsExist(IgniteEx ignite, CacheConfiguration ccfg) { + for (File dir : cacheStorageDirs(ignite, ccfg)) + assertTrue("Cache storage directory must exist: " + dir, dir.exists()); + } + + /** + * @param dirs Storage directories to await removal of. + */ + private void awaitStorageDirsRemoved(final File... dirs) throws Exception { + boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 30_000); + + assertTrue("Cache storage directories must be removed after destroy: " + Arrays.toString(dirs), res); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testDestroyStandaloneCacheRemovesDirectory() throws Exception { + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME); + + IgniteCache cache = ignite.createCache(ccfg); + + File[] dirs = cacheStorageDirs(ignite, ccfg); + + assertStorageDirsExist(ignite, ccfg); + + cache.destroy(); + + awaitStorageDirsRemoved(dirs); + } + } + + /** + * Checks that the shared group directory is removed only when the last cache of the group is destroyed. + * + * @throws Exception If failed. + */ + @Test + public void testDestroySharedCacheGroupRemovesDirectoryOnlyWhenEmpty() throws Exception { + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg1 = new CacheConfiguration<>(CACHE_1).setGroupName(GROUP_NAME); + CacheConfiguration ccfg2 = new CacheConfiguration<>(CACHE_2).setGroupName(GROUP_NAME); + + ignite.createCache(ccfg1); + ignite.createCache(ccfg2); + + File[] dirs = cacheStorageDirs(ignite, ccfg1); + + assertStorageDirsExist(ignite, ccfg1); + + ignite.cache(CACHE_1).destroy(); + + awaitPartitionMapExchange(); + + // The group still has another cache, so its storage directory must be kept. + assertTrue("Shared group storage directory must stay while other caches remain: " + Arrays.toString(dirs), + Arrays.stream(dirs).allMatch(File::exists)); + + ignite.cache(CACHE_2).destroy(); + + awaitStorageDirsRemoved(dirs); + } + } + + /** + * Checks that all storage directories (including an extra storage and a dedicated index storage) are removed after + * destroy. + * + * @throws Exception If failed. + */ + @Test + public void testDestroyRemovesAllStorageDirectories() throws Exception { + extraStorage = new File(U.defaultWorkDirectory(), "extra_storage"); + indexPath = new File(U.defaultWorkDirectory(), "index_storage"); + + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME) + .setStoragePaths(extraStorage.getAbsolutePath()) + .setIndexPath(indexPath.getAbsolutePath()); + + IgniteCache cache = ignite.createCache(ccfg); + + File[] dirs = cacheStorageDirs(ignite, ccfg); + + assertStorageDirsExist(ignite, ccfg); + + cache.destroy(); + + awaitStorageDirsRemoved(dirs); + } + } +} From a171cfcc3e4eda096359b81789e78cedaa4baee4 Mon Sep 17 00:00:00 2001 From: didar shayarov Date: Mon, 31 Aug 2026 00:54:10 +0300 Subject: [PATCH 2/4] IGNITE-13989 Destroy of persisted cache doesn't remove cache folder --- .../cache/GridLocalConfigManager.java | 23 ++ .../IgnitePdsDestroyRemovesCacheDirTest.java | 220 ++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java index 324849ec7af79..a7eab04ab60f9 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java @@ -24,7 +24,9 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.io.OutputStream; +import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayList; @@ -412,6 +414,27 @@ public void removeCacheGroupConfigurationData(CacheGroupContext ctx) throws Igni throw new IgniteCheckedException("Failed to delete cache configurations of group: " + ctx); } } + + // Remove leftover (now empty) cache storage directories so that PDS is not cluttered with empty + // directories after cache group destroy, see IGNITE-13989. + for (File dir : ft.cacheStorages(ctx.config())) { + if (!dir.exists()) + continue; + + try { + Files.delete(dir.toPath()); + } + catch (NoSuchFileException ignored) { + // Directory has already been removed by someone else. + } + catch (DirectoryNotEmptyException e) { + log.warning("Cache storage directory is not empty and cannot be removed: " + dir.getAbsolutePath()); + } + catch (IOException e) { + log.warning("Failed to remove cache storage directory [dir=" + dir.getAbsolutePath() + + ", reason=" + e.getMessage() + ']'); + } + } } /** diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java new file mode 100644 index 0000000000000..0d75b5cae10db --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java @@ -0,0 +1,220 @@ +/* + * 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; + +import java.io.File; +import java.util.Arrays; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.cluster.ClusterState; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.DataRegionConfiguration; +import org.apache.ignite.configuration.DataStorageConfiguration; +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.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +/** + * Tests that {@link IgniteCache#destroy()} removes the (empty) cache storage directories from the persistent storage, + * see IGNITE-13989. + */ +public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest { + /** Cache name for the shared cache group tests. */ + private static final String CACHE_1 = "cache-1"; + + /** Cache name for the shared cache group tests. */ + private static final String CACHE_2 = "cache-2"; + + /** Cache group name. */ + private static final String GROUP_NAME = "grp"; + + /** Additional storage path for multi-storage test. */ + private File extraStorage; + + /** Separate index path for index storage test. */ + private File indexPath; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setConsistentId(igniteInstanceName); + + DataStorageConfiguration dsCfg = new DataStorageConfiguration() + .setDefaultDataRegionConfiguration( + new DataRegionConfiguration().setPersistenceEnabled(true) + ); + + if (extraStorage != null) + dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath()); + + if (indexPath != null) { + // The index storage must be one of the DataStorageConfiguration storage paths, otherwise cache + // start validation fails. + if (extraStorage != null) + dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath(), indexPath.getAbsolutePath()); + else + dsCfg.setExtraStoragePaths(indexPath.getAbsolutePath()); + } + + cfg.setDataStorageConfiguration(dsCfg); + + return cfg; + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + stopAllGrids(); + + cleanPersistenceDir(); + + extraStorage = null; + indexPath = null; + } + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + stopAllGrids(); + + cleanPersistenceDir(); + + if (extraStorage != null) + U.delete(extraStorage); + + if (indexPath != null) + U.delete(indexPath); + + super.afterTest(); + } + + /** + * @param ignite Node. + * @param ccfg Cache configuration. + * @return Storage directories of the cache group. + */ + private File[] cacheStorageDirs(IgniteEx ignite, CacheConfiguration ccfg) { + return ignite.context().pdsFolderResolver().fileTree().cacheStorages(ccfg); + } + + /** + * @param ignite Node. + * @param ccfg Cache configuration. + */ + private void assertStorageDirsExist(IgniteEx ignite, CacheConfiguration ccfg) { + for (File dir : cacheStorageDirs(ignite, ccfg)) + assertTrue("Cache storage directory must exist: " + dir, dir.exists()); + } + + /** + * @param dirs Storage directories to await removal of. + */ + private void awaitStorageDirsRemoved(final File... dirs) throws Exception { + boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 30_000); + + assertTrue("Cache storage directories must be removed after destroy: " + Arrays.toString(dirs), res); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testDestroyStandaloneCacheRemovesDirectory() throws Exception { + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME); + + IgniteCache cache = ignite.createCache(ccfg); + + File[] dirs = cacheStorageDirs(ignite, ccfg); + + assertStorageDirsExist(ignite, ccfg); + + cache.destroy(); + + awaitStorageDirsRemoved(dirs); + } + } + + /** + * Checks that the shared group directory is removed only when the last cache of the group is destroyed. + * + * @throws Exception If failed. + */ + @Test + public void testDestroySharedCacheGroupRemovesDirectoryOnlyWhenEmpty() throws Exception { + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg1 = new CacheConfiguration<>(CACHE_1).setGroupName(GROUP_NAME); + CacheConfiguration ccfg2 = new CacheConfiguration<>(CACHE_2).setGroupName(GROUP_NAME); + + ignite.createCache(ccfg1); + ignite.createCache(ccfg2); + + File[] dirs = cacheStorageDirs(ignite, ccfg1); + + assertStorageDirsExist(ignite, ccfg1); + + ignite.cache(CACHE_1).destroy(); + + awaitPartitionMapExchange(); + + // The group still has another cache, so its storage directory must be kept. + assertTrue("Shared group storage directory must stay while other caches remain: " + Arrays.toString(dirs), + Arrays.stream(dirs).allMatch(File::exists)); + + ignite.cache(CACHE_2).destroy(); + + awaitStorageDirsRemoved(dirs); + } + } + + /** + * Checks that all storage directories (including an extra storage and a dedicated index storage) are removed after + * destroy. + * + * @throws Exception If failed. + */ + @Test + public void testDestroyRemovesAllStorageDirectories() throws Exception { + extraStorage = new File(U.defaultWorkDirectory(), "extra_storage"); + indexPath = new File(U.defaultWorkDirectory(), "index_storage"); + + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME) + .setStoragePaths(extraStorage.getAbsolutePath()) + .setIndexPath(indexPath.getAbsolutePath()); + + IgniteCache cache = ignite.createCache(ccfg); + + File[] dirs = cacheStorageDirs(ignite, ccfg); + + assertStorageDirsExist(ignite, ccfg); + + cache.destroy(); + + awaitStorageDirsRemoved(dirs); + } + } +} From 82ee133932ab9b9169a10220fc8037eb14f0bc65 Mon Sep 17 00:00:00 2001 From: didar shayarov Date: Mon, 31 Aug 2026 02:02:57 +0300 Subject: [PATCH 3/4] IGNITE-13989 Destroy of persisted cache doesn't remove cache folder --- .../cache/GridLocalConfigManager.java | 11 +- .../IgnitePdsDestroyRemovesCacheDirTest.java | 196 ++++++++++++++---- .../ignite/testsuites/IgnitePdsTestSuite.java | 2 + 3 files changed, 159 insertions(+), 50 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java index a7eab04ab60f9..87ca726de3d67 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java @@ -26,7 +26,6 @@ import java.io.OutputStream; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayList; @@ -416,16 +415,10 @@ public void removeCacheGroupConfigurationData(CacheGroupContext ctx) throws Igni } // Remove leftover (now empty) cache storage directories so that PDS is not cluttered with empty - // directories after cache group destroy, see IGNITE-13989. + // directories after cache group destroy, see IGNITE-13989. Non-empty directories are left untouched. for (File dir : ft.cacheStorages(ctx.config())) { - if (!dir.exists()) - continue; - try { - Files.delete(dir.toPath()); - } - catch (NoSuchFileException ignored) { - // Directory has already been removed by someone else. + Files.deleteIfExists(dir.toPath()); } catch (DirectoryNotEmptyException e) { log.warning("Cache storage directory is not empty and cannot be removed: " + dir.getAbsolutePath()); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java index 0d75b5cae10db..0699783931b98 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java @@ -18,6 +18,9 @@ import java.io.File; import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; import org.apache.ignite.IgniteCache; import org.apache.ignite.cluster.ClusterState; import org.apache.ignite.configuration.CacheConfiguration; @@ -31,10 +34,13 @@ import org.junit.Test; /** - * Tests that {@link IgniteCache#destroy()} removes the (empty) cache storage directories from the persistent storage, - * see IGNITE-13989. + * Tests that {@link IgniteCache#destroy()} removes the (now empty) cache storage directories from the persistent + * storage, see IGNITE-13989. */ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest { + /** Cache name used for most of the tests. */ + private static final String CACHE_NAME = "cache"; + /** Cache name for the shared cache group tests. */ private static final String CACHE_1 = "cache-1"; @@ -44,11 +50,14 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest /** Cache group name. */ private static final String GROUP_NAME = "grp"; - /** Additional storage path for multi-storage test. */ - private File extraStorage; + /** Number of keys written before destroy to make the cache persistent-backed. */ + private static final int KEYS_CNT = 1000; - /** Separate index path for index storage test. */ - private File indexPath; + /** + * Unique per-test roots of the extra storage paths ({@code DataStorageConfiguration#setExtraStoragePaths}). + * {@code Null} when a test uses only the default storage. + */ + private List extraStorages; /** {@inheritDoc} */ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { @@ -61,16 +70,10 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest new DataRegionConfiguration().setPersistenceEnabled(true) ); - if (extraStorage != null) - dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath()); + if (extraStorages != null) { + String[] extra = extraStorages.stream().map(File::getAbsolutePath).toArray(String[]::new); - if (indexPath != null) { - // The index storage must be one of the DataStorageConfiguration storage paths, otherwise cache - // start validation fails. - if (extraStorage != null) - dsCfg.setExtraStoragePaths(extraStorage.getAbsolutePath(), indexPath.getAbsolutePath()); - else - dsCfg.setExtraStoragePaths(indexPath.getAbsolutePath()); + dsCfg.setExtraStoragePaths(extra); } cfg.setDataStorageConfiguration(dsCfg); @@ -86,8 +89,7 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest cleanPersistenceDir(); - extraStorage = null; - indexPath = null; + extraStorages = null; } /** {@inheritDoc} */ @@ -96,11 +98,8 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest cleanPersistenceDir(); - if (extraStorage != null) - U.delete(extraStorage); - - if (indexPath != null) - U.delete(indexPath); + if (extraStorages != null) + extraStorages.forEach(U::delete); super.afterTest(); } @@ -108,28 +107,57 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest /** * @param ignite Node. * @param ccfg Cache configuration. - * @return Storage directories of the cache group. + * @return Storage directories of the cache group on the given node. */ private File[] cacheStorageDirs(IgniteEx ignite, CacheConfiguration ccfg) { return ignite.context().pdsFolderResolver().fileTree().cacheStorages(ccfg); } /** + * Asserts that every cache storage directory exists on the given node. + * * @param ignite Node. * @param ccfg Cache configuration. */ private void assertStorageDirsExist(IgniteEx ignite, CacheConfiguration ccfg) { - for (File dir : cacheStorageDirs(ignite, ccfg)) - assertTrue("Cache storage directory must exist: " + dir, dir.exists()); + File[] dirs = cacheStorageDirs(ignite, ccfg); + + for (File dir : dirs) + assertTrue("Cache storage directory must exist [node=" + ignite.name() + ", dir=" + dir + ']', + dir.exists()); } /** - * @param dirs Storage directories to await removal of. + * Asserts that the given cache storage directories have been removed. {@code cache.destroy()} is synchronous, so + * the files are expected to be gone right after it returns; a short bounded poll is used only to absorb any + * remaining asynchronous fs activity. + * + * @param dirs Storage directories to assert removal of. */ - private void awaitStorageDirsRemoved(final File... dirs) throws Exception { - boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 30_000); + private void assertStorageDirsRemoved(final File... dirs) throws Exception { + boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 5_000); - assertTrue("Cache storage directories must be removed after destroy: " + Arrays.toString(dirs), res); + if (!res) { + String remaining = Arrays.stream(dirs) + .map(File::getAbsolutePath) + .collect(Collectors.joining(", ")); + + fail("Cache storage directories must be removed after destroy, remaining: " + remaining); + } + } + + /** + * Puts some data into the given cache and forces a checkpoint so that partition/index files are actually written + * to the persistent storage before {@code destroy()}. + * + * @param ignite Node. + * @param cache Cache. + */ + private void loadDataAndCheckpoint(IgniteEx ignite, IgniteCache cache) throws Exception { + for (int i = 0; i < KEYS_CNT; i++) + cache.put(i, "value-" + i); + + forceCheckpoint(ignite); } /** @@ -140,17 +168,19 @@ public void testDestroyStandaloneCacheRemovesDirectory() throws Exception { try (IgniteEx ignite = startGrid(0)) { ignite.cluster().state(ClusterState.ACTIVE); - CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); IgniteCache cache = ignite.createCache(ccfg); + loadDataAndCheckpoint(ignite, cache); + File[] dirs = cacheStorageDirs(ignite, ccfg); assertStorageDirsExist(ignite, ccfg); cache.destroy(); - awaitStorageDirsRemoved(dirs); + assertStorageDirsRemoved(dirs); } } @@ -179,42 +209,126 @@ public void testDestroySharedCacheGroupRemovesDirectoryOnlyWhenEmpty() throws Ex awaitPartitionMapExchange(); // The group still has another cache, so its storage directory must be kept. - assertTrue("Shared group storage directory must stay while other caches remain: " + Arrays.toString(dirs), - Arrays.stream(dirs).allMatch(File::exists)); + for (File dir : dirs) + assertTrue("Shared group storage directory must stay while other caches remain [dir=" + dir + ']', + dir.exists()); ignite.cache(CACHE_2).destroy(); - awaitStorageDirsRemoved(dirs); + assertStorageDirsRemoved(dirs); } } /** - * Checks that all storage directories (including an extra storage and a dedicated index storage) are removed after + * Checks that all storage directories (two data storage paths plus a distinct index path) are removed after * destroy. * * @throws Exception If failed. */ @Test public void testDestroyRemovesAllStorageDirectories() throws Exception { - extraStorage = new File(U.defaultWorkDirectory(), "extra_storage"); - indexPath = new File(U.defaultWorkDirectory(), "index_storage"); + extraStorages = Arrays.asList(newUniqueStorageDir(), newUniqueStorageDir(), newUniqueStorageDir()); + + try (IgniteEx ignite = startGrid(0)) { + ignite.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) + .setStoragePaths(extraStorages.get(0).getAbsolutePath(), extraStorages.get(1).getAbsolutePath()) + .setIndexPath(extraStorages.get(2).getAbsolutePath()); + + IgniteCache cache = ignite.createCache(ccfg); + + loadDataAndCheckpoint(ignite, cache); + + File[] dirs = cacheStorageDirs(ignite, ccfg); + + // Two data storages plus one index storage. + assertEquals("Unexpected number of storage directories", 3, dirs.length); + + assertStorageDirsExist(ignite, ccfg); + + cache.destroy(); + + assertStorageDirsRemoved(dirs); + } + } + /** + * Checks that the storage directories are removed on both server nodes of a two-node cluster after destroy. + * + * @throws Exception If failed. + */ + @Test + public void testDestroyOnTwoServersRemovesDirectoriesOnBothNodes() throws Exception { + try (IgniteEx ignite0 = startGrid(0); IgniteEx ignite1 = startGrid(1)) { + ignite0.cluster().state(ClusterState.ACTIVE); + + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) + .setBackups(1); + + IgniteCache cache = ignite0.createCache(ccfg); + + loadDataAndCheckpoint(ignite0, cache); + + File[] dirs0 = cacheStorageDirs(ignite0, ccfg); + File[] dirs1 = cacheStorageDirs(ignite1, ccfg); + + assertStorageDirsExist(ignite0, ccfg); + assertStorageDirsExist(ignite1, ccfg); + + cache.destroy(); + + assertStorageDirsRemoved(dirs0); + assertStorageDirsRemoved(dirs1); + } + } + + /** + * Checks that a cache of the same name can be immediately recreated after destroy and remains functional. + * + * @throws Exception If failed. + */ + @Test + public void testDestroyAndRecreateCache() throws Exception { try (IgniteEx ignite = startGrid(0)) { ignite.cluster().state(ClusterState.ACTIVE); - CacheConfiguration ccfg = new CacheConfiguration<>(DEFAULT_CACHE_NAME) - .setStoragePaths(extraStorage.getAbsolutePath()) - .setIndexPath(indexPath.getAbsolutePath()); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); IgniteCache cache = ignite.createCache(ccfg); + loadDataAndCheckpoint(ignite, cache); + File[] dirs = cacheStorageDirs(ignite, ccfg); assertStorageDirsExist(ignite, ccfg); cache.destroy(); - awaitStorageDirsRemoved(dirs); + assertStorageDirsRemoved(dirs); + + // Immediately recreate a cache with the same name. + IgniteCache newCache = ignite.createCache(ccfg); + + assertStorageDirsExist(ignite, ccfg); + + newCache.put(1, 1); + + assertEquals(1, newCache.get(1)); + + // The recreated cache must be able to persist and read back its own data. + forceCheckpoint(ignite); + + newCache.put(2, 2); + + assertEquals(2, newCache.get(2)); } } + + /** + * @return A unique, per-test directory under the default work directory to be used as an external storage root. + */ + private File newUniqueStorageDir() throws Exception { + return new File(U.defaultWorkDirectory(), getClass().getSimpleName() + "-" + UUID.randomUUID()); + } } diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgnitePdsTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgnitePdsTestSuite.java index 95b29c84e93de..d0ee8b9a5b53e 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgnitePdsTestSuite.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgnitePdsTestSuite.java @@ -30,6 +30,7 @@ import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsClientNearCachePutGetTest; import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsDestroyCacheTest; import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsDestroyCacheWithoutCheckpointsTest; +import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsDestroyRemovesCacheDirTest; import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsDynamicCacheTest; import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsRemoveDuringRebalancingTest; import org.apache.ignite.internal.processors.cache.persistence.IgnitePdsSingleNodePutGetPersistenceTest; @@ -129,6 +130,7 @@ public static void addRealPageStoreTests(List> suite, Collection GridTestUtils.addTestIfNeeded(suite, IgnitePdsDataRegionMetricsTxTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, IgnitePdsDestroyCacheTest.class, ignoredTests); + GridTestUtils.addTestIfNeeded(suite, IgnitePdsDestroyRemovesCacheDirTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, IgnitePdsRemoveDuringRebalancingTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, IgnitePdsDestroyCacheWithoutCheckpointsTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, IgnitePdsCacheConfigurationFileConsistencyCheckTest.class, ignoredTests); From 90660c44f78ff5027bc3ce6d224f12449722a7d8 Mon Sep 17 00:00:00 2001 From: didar shayarov Date: Mon, 31 Aug 2026 21:27:27 +0300 Subject: [PATCH 4/4] IGNITE-13989 Fix tests --- .../cache/GridLocalConfigManager.java | 3 +- .../IgnitePdsDestroyRemovesCacheDirTest.java | 328 ++++++++++-------- 2 files changed, 193 insertions(+), 138 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java index 87ca726de3d67..91326fcdea1a3 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridLocalConfigManager.java @@ -424,8 +424,7 @@ public void removeCacheGroupConfigurationData(CacheGroupContext ctx) throws Igni log.warning("Cache storage directory is not empty and cannot be removed: " + dir.getAbsolutePath()); } catch (IOException e) { - log.warning("Failed to remove cache storage directory [dir=" + dir.getAbsolutePath() + - ", reason=" + e.getMessage() + ']'); + log.warning("Failed to remove cache storage directory [dir=" + dir.getAbsolutePath() + ']', e); } } } diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java index 0699783931b98..9152d9521f8f5 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsDestroyRemovesCacheDirTest.java @@ -14,13 +14,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.apache.ignite.internal.processors.cache.persistence; import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.List; import java.util.UUID; -import java.util.stream.Collectors; import org.apache.ignite.IgniteCache; import org.apache.ignite.cluster.ClusterState; import org.apache.ignite.configuration.CacheConfiguration; @@ -50,9 +52,6 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest /** Cache group name. */ private static final String GROUP_NAME = "grp"; - /** Number of keys written before destroy to make the cache persistent-backed. */ - private static final int KEYS_CNT = 1000; - /** * Unique per-test roots of the extra storage paths ({@code DataStorageConfiguration#setExtraStoragePaths}). * {@code Null} when a test uses only the default storage. @@ -85,8 +84,6 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest @Override protected void beforeTest() throws Exception { super.beforeTest(); - stopAllGrids(); - cleanPersistenceDir(); extraStorages = null; @@ -105,224 +102,283 @@ public class IgnitePdsDestroyRemovesCacheDirTest extends GridCommonAbstractTest } /** - * @param ignite Node. - * @param ccfg Cache configuration. - * @return Storage directories of the cache group on the given node. + * @throws Exception If failed. */ - private File[] cacheStorageDirs(IgniteEx ignite, CacheConfiguration ccfg) { - return ignite.context().pdsFolderResolver().fileTree().cacheStorages(ccfg); - } + @Test + public void testDestroyStandaloneCacheRemovesDirectory() throws Exception { + IgniteEx ignite = startActiveGrid(0); - /** - * Asserts that every cache storage directory exists on the given node. - * - * @param ignite Node. - * @param ccfg Cache configuration. - */ - private void assertStorageDirsExist(IgniteEx ignite, CacheConfiguration ccfg) { - File[] dirs = cacheStorageDirs(ignite, ccfg); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); - for (File dir : dirs) - assertTrue("Cache storage directory must exist [node=" + ignite.name() + ", dir=" + dir + ']', - dir.exists()); - } + IgniteCache cache = ignite.createCache(ccfg); - /** - * Asserts that the given cache storage directories have been removed. {@code cache.destroy()} is synchronous, so - * the files are expected to be gone right after it returns; a short bounded poll is used only to absorb any - * remaining asynchronous fs activity. - * - * @param dirs Storage directories to assert removal of. - */ - private void assertStorageDirsRemoved(final File... dirs) throws Exception { - boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 5_000); + persistEntry(ignite, cache); - if (!res) { - String remaining = Arrays.stream(dirs) - .map(File::getAbsolutePath) - .collect(Collectors.joining(", ")); + File[] dirs = cacheStorageDirs(ignite, ccfg); - fail("Cache storage directories must be removed after destroy, remaining: " + remaining); - } - } + assertStorageDirsExist(dirs); - /** - * Puts some data into the given cache and forces a checkpoint so that partition/index files are actually written - * to the persistent storage before {@code destroy()}. - * - * @param ignite Node. - * @param cache Cache. - */ - private void loadDataAndCheckpoint(IgniteEx ignite, IgniteCache cache) throws Exception { - for (int i = 0; i < KEYS_CNT; i++) - cache.put(i, "value-" + i); + cache.destroy(); - forceCheckpoint(ignite); + assertStorageDirsRemoved(dirs); } /** + * Checks that the shared group directory is removed only when the last cache of the group is destroyed. + * * @throws Exception If failed. */ @Test - public void testDestroyStandaloneCacheRemovesDirectory() throws Exception { - try (IgniteEx ignite = startGrid(0)) { - ignite.cluster().state(ClusterState.ACTIVE); + public void testDestroySharedCacheGroupRemovesDirectoryOnlyWhenEmpty() throws Exception { + IgniteEx ignite = startActiveGrid(0); - CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); + CacheConfiguration ccfg1 = new CacheConfiguration<>(CACHE_1).setGroupName(GROUP_NAME); + CacheConfiguration ccfg2 = new CacheConfiguration<>(CACHE_2).setGroupName(GROUP_NAME); - IgniteCache cache = ignite.createCache(ccfg); + ignite.createCache(ccfg1); + ignite.createCache(ccfg2); - loadDataAndCheckpoint(ignite, cache); + File[] dirs = cacheStorageDirs(ignite, ccfg1); - File[] dirs = cacheStorageDirs(ignite, ccfg); + assertStorageDirsExist(dirs); - assertStorageDirsExist(ignite, ccfg); + ignite.cache(CACHE_1).destroy(); - cache.destroy(); + // The group still has another cache, so its storage directory must be kept. + assertStorageDirsExist(dirs); - assertStorageDirsRemoved(dirs); - } + ignite.cache(CACHE_2).destroy(); + + assertStorageDirsRemoved(dirs); } /** - * Checks that the shared group directory is removed only when the last cache of the group is destroyed. + * Checks that all storage directories (two data storage paths plus a distinct index path) are removed after + * destroy. * * @throws Exception If failed. */ @Test - public void testDestroySharedCacheGroupRemovesDirectoryOnlyWhenEmpty() throws Exception { - try (IgniteEx ignite = startGrid(0)) { - ignite.cluster().state(ClusterState.ACTIVE); + public void testDestroyRemovesAllStorageDirectories() throws Exception { + extraStorages = Arrays.asList(newUniqueStorageDir(), newUniqueStorageDir(), newUniqueStorageDir()); - CacheConfiguration ccfg1 = new CacheConfiguration<>(CACHE_1).setGroupName(GROUP_NAME); - CacheConfiguration ccfg2 = new CacheConfiguration<>(CACHE_2).setGroupName(GROUP_NAME); + IgniteEx ignite = startActiveGrid(0); - ignite.createCache(ccfg1); - ignite.createCache(ccfg2); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) + .setStoragePaths(extraStorages.get(0).getAbsolutePath(), extraStorages.get(1).getAbsolutePath()) + .setIndexPath(extraStorages.get(2).getAbsolutePath()); - File[] dirs = cacheStorageDirs(ignite, ccfg1); + IgniteCache cache = ignite.createCache(ccfg); - assertStorageDirsExist(ignite, ccfg1); + // Storage selection for a data partition is storages[partition % storages.length], so a key of an even + // partition lands in the first storage path and a key of an odd one in the second. Pick one key per path so + // that both data storages are deterministically populated. + List evenPartKeys = partitionKeys(cache, 0, 1, 0); + List oddPartKeys = partitionKeys(cache, 1, 1, 0); - ignite.cache(CACHE_1).destroy(); + cache.put(evenPartKeys.get(0), 1); + cache.put(oddPartKeys.get(0), 2); - awaitPartitionMapExchange(); + forceCheckpoint(ignite); - // The group still has another cache, so its storage directory must be kept. - for (File dir : dirs) - assertTrue("Shared group storage directory must stay while other caches remain [dir=" + dir + ']', - dir.exists()); + File[] dirs = cacheStorageDirs(ignite, ccfg); - ignite.cache(CACHE_2).destroy(); + // Two data storages plus one index storage. + assertEquals("Unexpected number of storage directories", 3, dirs.length); - assertStorageDirsRemoved(dirs); - } + assertStorageDirsExist(dirs); + + // Each of the configured storage directories must actually have been used as a page-store location, not just + // created and thereafter deleted as an empty directory. + assertTrue("Every configured storage directory must contain a page-store file", + Arrays.stream(dirs).allMatch(IgnitePdsDestroyRemovesCacheDirTest::containsPageStoreFile)); + + cache.destroy(); + + assertStorageDirsRemoved(dirs); } /** - * Checks that all storage directories (two data storage paths plus a distinct index path) are removed after - * destroy. + * Checks that the storage directories are removed on both server nodes of a two-node cluster after destroy. * * @throws Exception If failed. */ @Test - public void testDestroyRemovesAllStorageDirectories() throws Exception { - extraStorages = Arrays.asList(newUniqueStorageDir(), newUniqueStorageDir(), newUniqueStorageDir()); + public void testDestroyOnTwoServersRemovesDirectoriesOnBothNodes() throws Exception { + // Start both nodes first and only then activate the cluster so that both nodes enter the baseline and both + // actually store data. A node that is started after activation and thus not in the baseline does not store + // data and cleans its storage directory asynchronously, which would make the synchronous assertion below + // flaky. + IgniteEx ignite0 = startGrid(0); + IgniteEx ignite1 = startGrid(1); - try (IgniteEx ignite = startGrid(0)) { - ignite.cluster().state(ClusterState.ACTIVE); + ignite0.cluster().state(ClusterState.ACTIVE); - CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) - .setStoragePaths(extraStorages.get(0).getAbsolutePath(), extraStorages.get(1).getAbsolutePath()) - .setIndexPath(extraStorages.get(2).getAbsolutePath()); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) + .setBackups(1); - IgniteCache cache = ignite.createCache(ccfg); + IgniteCache cache = ignite0.createCache(ccfg); - loadDataAndCheckpoint(ignite, cache); + persistEntry(ignite0, cache); - File[] dirs = cacheStorageDirs(ignite, ccfg); + File[] dirs0 = cacheStorageDirs(ignite0, ccfg); + File[] dirs1 = cacheStorageDirs(ignite1, ccfg); - // Two data storages plus one index storage. - assertEquals("Unexpected number of storage directories", 3, dirs.length); + assertStorageDirsExist(dirs0); + assertStorageDirsExist(dirs1); - assertStorageDirsExist(ignite, ccfg); + cache.destroy(); - cache.destroy(); + // On the initiating node the directory is removed synchronously as part of the destroy exchange. + assertStorageDirsRemoved(dirs0); - assertStorageDirsRemoved(dirs); - } + // On a remote server the local directory cleanup runs in its own exchange thread and may not be finished by + // the time destroy() returns on the initiating node, so a bounded wait is required here. + assertStorageDirsRemovedEventually(dirs1); } /** - * Checks that the storage directories are removed on both server nodes of a two-node cluster after destroy. + * Checks that a cache of the same name can be immediately recreated after destroy and remains functional. * * @throws Exception If failed. */ @Test - public void testDestroyOnTwoServersRemovesDirectoriesOnBothNodes() throws Exception { - try (IgniteEx ignite0 = startGrid(0); IgniteEx ignite1 = startGrid(1)) { - ignite0.cluster().state(ClusterState.ACTIVE); + public void testCacheCanBeRecreatedAfterDirectoryRemoval() throws Exception { + IgniteEx ignite = startActiveGrid(0); - CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME) - .setBackups(1); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); - IgniteCache cache = ignite0.createCache(ccfg); + IgniteCache cache = ignite.createCache(ccfg); - loadDataAndCheckpoint(ignite0, cache); + persistEntry(ignite, cache); - File[] dirs0 = cacheStorageDirs(ignite0, ccfg); - File[] dirs1 = cacheStorageDirs(ignite1, ccfg); + File[] dirs = cacheStorageDirs(ignite, ccfg); - assertStorageDirsExist(ignite0, ccfg); - assertStorageDirsExist(ignite1, ccfg); + assertStorageDirsExist(dirs); - cache.destroy(); + cache.destroy(); - assertStorageDirsRemoved(dirs0); - assertStorageDirsRemoved(dirs1); - } + assertStorageDirsRemoved(dirs); + + // Immediately recreate a cache with the same name. + IgniteCache newCache = ignite.createCache(ccfg); + + File[] newDirs = cacheStorageDirs(ignite, ccfg); + + assertStorageDirsExist(newDirs); + + newCache.put(1, 1); + + assertEquals(1, newCache.get(1)); } /** - * Checks that a cache of the same name can be immediately recreated after destroy and remains functional. + * Checks that a non-empty storage directory is preserved on destroy: the fix removes only empty leftover + * directories and never deletes unrelated files. * * @throws Exception If failed. */ @Test - public void testDestroyAndRecreateCache() throws Exception { - try (IgniteEx ignite = startGrid(0)) { - ignite.cluster().state(ClusterState.ACTIVE); + public void testDestroyKeepsNonEmptyStorageDirectory() throws Exception { + IgniteEx ignite = startActiveGrid(0); - CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); + CacheConfiguration ccfg = new CacheConfiguration<>(CACHE_NAME); - IgniteCache cache = ignite.createCache(ccfg); + IgniteCache cache = ignite.createCache(ccfg); - loadDataAndCheckpoint(ignite, cache); + persistEntry(ignite, cache); - File[] dirs = cacheStorageDirs(ignite, ccfg); + File dir = cacheStorageDirs(ignite, ccfg)[0]; - assertStorageDirsExist(ignite, ccfg); + Path marker = dir.toPath().resolve("do-not-delete"); - cache.destroy(); + Files.write(marker, new byte[] {1}); - assertStorageDirsRemoved(dirs); + cache.destroy(); - // Immediately recreate a cache with the same name. - IgniteCache newCache = ignite.createCache(ccfg); + assertTrue("Non-empty storage directory must be preserved", dir.exists()); + assertTrue("Foreign file must not be removed", Files.exists(marker)); + } - assertStorageDirsExist(ignite, ccfg); + /** + * Starts a grid node and activates the cluster. + * + * @param idx Node index. + * @return Started node. + */ + private IgniteEx startActiveGrid(int idx) throws Exception { + IgniteEx ignite = startGrid(idx); - newCache.put(1, 1); + ignite.cluster().state(ClusterState.ACTIVE); - assertEquals(1, newCache.get(1)); + return ignite; + } - // The recreated cache must be able to persist and read back its own data. - forceCheckpoint(ignite); + /** + * Puts a single entry into the given cache and forces a checkpoint so that partition/index files are actually + * written to the persistent storage before {@code destroy()}. + * + * @param ignite Node. + * @param cache Cache. + */ + private void persistEntry(IgniteEx ignite, IgniteCache cache) throws Exception { + cache.put(0, 0); - newCache.put(2, 2); + forceCheckpoint(ignite); + } - assertEquals(2, newCache.get(2)); - } + /** + * @param ignite Node. + * @param ccfg Cache configuration. + * @return Storage directories of the cache group on the given node. + */ + private File[] cacheStorageDirs(IgniteEx ignite, CacheConfiguration ccfg) { + return ignite.context().pdsFolderResolver().fileTree().cacheStorages(ccfg); + } + + /** + * Asserts that every cache storage directory exists. + * + * @param dirs Storage directories. + */ + private void assertStorageDirsExist(File... dirs) { + for (File dir : dirs) + assertTrue("Cache storage directory must exist [dir=" + dir + ']', dir.exists()); + } + + /** + * Asserts that the given cache storage directories have been removed. {@code cache.destroy()} is synchronous and + * shuts the page stores down before deleting the directories, so they are expected to be gone immediately after + * it returns. + * + * @param dirs Storage directories to assert removal of. + */ + private void assertStorageDirsRemoved(File... dirs) { + for (File dir : dirs) + assertFalse("Storage directory was not removed [dir=" + dir + ']', dir.exists()); + } + + /** + * Asserts that the given cache storage directories are eventually removed. Unlike the initiating node, a remote + * server performs its local directory cleanup in its own exchange thread, which may lag slightly behind the + * {@code destroy()} call, so a short bounded wait is used. + * + * @param dirs Storage directories to assert removal of. + */ + private void assertStorageDirsRemovedEventually(File... dirs) throws Exception { + boolean res = GridTestUtils.waitForCondition(() -> Arrays.stream(dirs).noneMatch(File::exists), 10_000); + + assertTrue("Storage directories were not removed [dirs=" + Arrays.toString(dirs) + ']', res); + } + + /** + * @param dir Directory. + * @return {@code True} if the directory is non-empty, i.e. it actually stored page-store files. + */ + private static boolean containsPageStoreFile(File dir) { + File[] files = dir.listFiles(); + + return files != null && files.length > 0; } /**