From 3f18c1db7e5d1515fb8587e418f3690616d5402a Mon Sep 17 00:00:00 2001 From: Sammi Chen Date: Wed, 24 Mar 2021 15:06:37 +0800 Subject: [PATCH 1/2] HDDS-4987. Import container should not delete container contents if container already exists --- .../container/keyvalue/KeyValueContainer.java | 11 +++++++-- .../keyvalue/TestKeyValueContainer.java | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 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 6760a27ce36f..20c5885f346d 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 @@ -466,7 +466,8 @@ public void importContainerData(InputStream input, + " as the container descriptor (%s) has already been exist.", getContainerData().getContainerID(), getContainerFile().getAbsolutePath()); - throw new IOException(errorMessage); + throw new StorageContainerException(errorMessage, + CONTAINER_ALREADY_EXISTS); } //copy the values from the input stream to the final destination // directory. @@ -496,11 +497,17 @@ public void importContainerData(InputStream input, KeyValueContainerUtil.parseKVContainerData(containerData, config); } catch (Exception ex) { + if (ex instanceof StorageContainerException && + ((StorageContainerException) ex).getResult() == + CONTAINER_ALREADY_EXISTS) { + throw ex; + } //delete all the temporary data in case of any exception. try { FileUtils.deleteDirectory(new File(containerData.getMetadataPath())); FileUtils.deleteDirectory(new File(containerData.getChunksPath())); - FileUtils.deleteDirectory(getContainerFile()); + FileUtils.deleteDirectory( + new File(getContainerData().getContainerPath())); } catch (Exception deleteex) { LOG.error( "Can not cleanup destination directories after a container import" 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 d50e240a4c65..06d38dacf1fd 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 @@ -206,8 +206,32 @@ public void testContainerImportExport() throws Exception { fail("Container is imported twice. Previous files are overwritten"); } catch (IOException ex) { //all good + assertTrue(container.getContainerFile().exists()); } + //Import failure should cleanup the container directory + containerData = + new KeyValueContainerData(containerId + 1, + keyValueContainerData.getLayOutVersion(), + keyValueContainerData.getMaxSize(), UUID.randomUUID().toString(), + datanodeId.toString()); + container = new KeyValueContainer(containerData, CONF); + + containerVolume = volumeChoosingPolicy.chooseVolume(volumeSet + .getVolumesList(), 1); + hddsVolumeDir = containerVolume.getHddsRootDir().toString(); + container.populatePathFields(scmId, containerVolume, hddsVolumeDir); + try (FileInputStream fis = new FileInputStream(folderToExport)) { + fis.close(); + container.importContainerData(fis, packer); + fail("Container import should fail"); + } catch (Exception ex) { + assertTrue(ex instanceof IOException); + } finally { + File directory = + new File(container.getContainerData().getContainerPath()); + assertFalse(directory.exists()); + } } /** From 15f66c8c7a05fea3185039ca64bd224a0336f660 Mon Sep 17 00:00:00 2001 From: Sammi Chen Date: Thu, 25 Mar 2021 14:15:57 +0800 Subject: [PATCH 2/2] address comment --- .../hadoop/ozone/container/keyvalue/TestKeyValueContainer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 06d38dacf1fd..8989c3e6b334 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 @@ -221,7 +221,8 @@ public void testContainerImportExport() throws Exception { .getVolumesList(), 1); hddsVolumeDir = containerVolume.getHddsRootDir().toString(); container.populatePathFields(scmId, containerVolume, hddsVolumeDir); - try (FileInputStream fis = new FileInputStream(folderToExport)) { + try { + FileInputStream fis = new FileInputStream(folderToExport); fis.close(); container.importContainerData(fis, packer); fail("Container import should fail");