From 90ff1009c79434ca6a1c3bda5c93fd9cf751c7f0 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Wed, 6 May 2026 13:01:45 +0530 Subject: [PATCH 1/2] HDDS-15038. Change cleanupFailedImport() to delete chunks before metadata to avoid chunks-only residual state. --- .../container/keyvalue/KeyValueContainer.java | 11 +++- .../keyvalue/TestKeyValueContainer.java | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java index b8214882f12e..26744ff423c5 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java @@ -664,15 +664,20 @@ private void cleanupFailedImport() { if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) { BlockUtils.removeContainerFromDB(containerData, config); } - FileUtils.deleteDirectory(new File(containerData.getMetadataPath())); - FileUtils.deleteDirectory(new File(containerData.getChunksPath())); - FileUtils.deleteDirectory(new File(getContainerData().getContainerPath())); + deleteDirectory(new File(containerData.getChunksPath())); + deleteDirectory(new File(containerData.getMetadataPath())); + deleteDirectory(new File(getContainerData().getContainerPath())); } catch (Exception ex) { LOG.error("Failed to cleanup destination directories for container {}", containerData.getContainerID(), ex); } } + @VisibleForTesting + void deleteDirectory(File directory) throws IOException { + FileUtils.deleteDirectory(directory); + } + @Override public void exportContainerData(OutputStream destination, ContainerPacker packer) throws IOException { diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 9e66aaeb067a..9ab7bbde4eeb 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -44,6 +44,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -79,6 +80,8 @@ import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml; import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion; +import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.interfaces.ContainerPacker; import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.hadoop.ozone.container.common.utils.DatanodeStoreCache; @@ -492,6 +495,58 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo) } } + @ContainerTestVersionInfo.ContainerTest + public void testFailedImportCleanupDeletesChunksBeforeMetadata( + ContainerTestVersionInfo versionInfo) throws Exception { + init(versionInfo); + + HddsVolume containerVolume = volumeChoosingPolicy.chooseVolume( + StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()), 1); + + KeyValueContainer container = new KeyValueContainer( + keyValueContainerData, CONF) { + @Override + void deleteDirectory(File directory) throws IOException { + if (directory.equals(new File(getContainerData().getMetadataPath()))) { + throw new IOException("Injected metadata cleanup failure"); + } + super.deleteDirectory(directory); + } + }; + container.populatePathFields(scmId, containerVolume); + + ContainerPacker failingPacker = + new ContainerPacker() { + @Override + public byte[] unpackContainerData( + Container containerToUnpack, + InputStream inputStream, Path tmpDir, Path destContainerDir) + throws IOException { + Files.createDirectories(new File(containerToUnpack + .getContainerData().getChunksPath()).toPath()); + Files.createDirectories(new File(containerToUnpack + .getContainerData().getMetadataPath()).toPath()); + throw new IOException("Injected import failure"); + } + + @Override + public void pack(Container containerToPack, + OutputStream destination) { + } + + @Override + public byte[] unpackContainerDescriptor(InputStream inputStream) { + return null; + } + }; + + assertThrows(IOException.class, () -> container.importContainerData( + new ByteArrayInputStream(new byte[0]), failingPacker)); + + assertFalse(new File(container.getContainerData().getChunksPath()).exists()); + assertTrue(new File(container.getContainerData().getMetadataPath()).exists()); + } + private void checkContainerFilesPresent(KeyValueContainerData data, long expectedNumFilesInChunksDir) throws IOException { File chunksDir = new File(data.getChunksPath()); From 9b392339167d3bd88c83c313711aa4a5e74bd254 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Fri, 8 May 2026 15:23:08 +0530 Subject: [PATCH 2/2] HDDS-15038. Changed deletion approach to move the container dir to tmp dir and then delete. --- .../container/keyvalue/KeyValueContainer.java | 10 +++++++--- .../keyvalue/TestKeyValueContainer.java | 20 ++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java index 26744ff423c5..5be6d20a336a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java @@ -664,9 +664,13 @@ private void cleanupFailedImport() { if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) { BlockUtils.removeContainerFromDB(containerData, config); } - deleteDirectory(new File(containerData.getChunksPath())); - deleteDirectory(new File(containerData.getMetadataPath())); - deleteDirectory(new File(getContainerData().getContainerPath())); + File containerDir = new File(getContainerData().getContainerPath()); + if (containerDir.exists()) { + KeyValueContainerUtil.moveToDeletedContainerDir(containerData, + containerData.getVolume()); + deleteDirectory(KeyValueContainerUtil.getTmpDirectoryPath(containerData, + containerData.getVolume()).toFile()); + } } catch (Exception ex) { LOG.error("Failed to cleanup destination directories for container {}", containerData.getContainerID(), ex); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 9ab7bbde4eeb..6fde5a454aa2 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -496,7 +496,7 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo) } @ContainerTestVersionInfo.ContainerTest - public void testFailedImportCleanupDeletesChunksBeforeMetadata( + public void testFailedImportCleanupMovesContainerBeforeDelete( ContainerTestVersionInfo versionInfo) throws Exception { init(versionInfo); @@ -507,8 +507,10 @@ public void testFailedImportCleanupDeletesChunksBeforeMetadata( keyValueContainerData, CONF) { @Override void deleteDirectory(File directory) throws IOException { - if (directory.equals(new File(getContainerData().getMetadataPath()))) { - throw new IOException("Injected metadata cleanup failure"); + File deletedContainerDir = KeyValueContainerUtil.getTmpDirectoryPath( + getContainerData(), getContainerData().getVolume()).toFile(); + if (directory.equals(deletedContainerDir)) { + throw new IOException("Injected tmp cleanup failure"); } super.deleteDirectory(directory); } @@ -543,8 +545,16 @@ public byte[] unpackContainerDescriptor(InputStream inputStream) { assertThrows(IOException.class, () -> container.importContainerData( new ByteArrayInputStream(new byte[0]), failingPacker)); - assertFalse(new File(container.getContainerData().getChunksPath()).exists()); - assertTrue(new File(container.getContainerData().getMetadataPath()).exists()); + assertFalse(new File(container.getContainerData().getContainerPath()) + .exists()); + File deletedContainerDir = KeyValueContainerUtil.getTmpDirectoryPath( + container.getContainerData(), container.getContainerData().getVolume()) + .toFile(); + assertTrue(deletedContainerDir.exists()); + assertTrue(new File(deletedContainerDir, OzoneConsts.STORAGE_DIR_CHUNKS) + .exists()); + assertTrue(new File(deletedContainerDir, OzoneConsts.CONTAINER_META_PATH) + .exists()); } private void checkContainerFilesPresent(KeyValueContainerData data,