This is an automated email from the ASF dual-hosted git repository.
erose pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 8b0cdfff74 HDDS-8770. Cleanup of failed container delete may remove
datanode RocksDB entries of active container (#4972)
8b0cdfff74 is described below
commit 8b0cdfff7410e878bf87ef0ba70c64647755fd5a
Author: Sumit Agrawal <[email protected]>
AuthorDate: Thu Jul 13 23:07:13 2023 +0530
HDDS-8770. Cleanup of failed container delete may remove datanode RocksDB
entries of active container (#4972)
---
.../container/common/interfaces/Container.java | 5 +
.../ozone/container/common/volume/HddsVolume.java | 56 ---------
.../container/keyvalue/KeyValueContainer.java | 27 ++++-
.../ozone/container/keyvalue/KeyValueHandler.java | 27 ++---
.../keyvalue/helpers/KeyValueContainerUtil.java | 82 ++++++-------
.../ozone/container/ozoneimpl/ContainerReader.java | 24 ++++
.../hadoop/ozone/TestHddsDatanodeService.java | 7 +-
.../ozone/container/common/ContainerTestUtils.java | 32 +++--
.../common/impl/TestContainerPersistence.java | 16 ++-
.../container/keyvalue/TestKeyValueContainer.java | 10 ++
.../container/keyvalue/TestKeyValueHandler.java | 11 +-
.../container/ozoneimpl/TestContainerReader.java | 63 ++++++++++
.../ozone/container/common/TestEndPoint.java | 130 ++++++++++-----------
13 files changed, 290 insertions(+), 200 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java
index de59f9bbea..6a9793a691 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java
@@ -160,6 +160,11 @@ public interface Container<CONTAINERDATA extends
ContainerData> extends RwLock {
*/
void markContainerUnhealthy() throws StorageContainerException;
+ /**
+ * Marks the container replica as deleted.
+ */
+ void markContainerForDelete();
+
/**
* Quasi Closes a open container, if it is already closed or does not exist a
* StorageContainerException is thrown.
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java
index 2a2de08cb2..581e5aaa43 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java
@@ -31,18 +31,11 @@ import org.apache.hadoop.hdds.annotation.InterfaceAudience;
import org.apache.hadoop.hdds.annotation.InterfaceStability;
import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature;
import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult;
-import org.apache.hadoop.ozone.OzoneConsts;
-import org.apache.hadoop.ozone.container.common.helpers.ContainerUtils;
-import org.apache.hadoop.ozone.container.common.impl.ContainerData;
-import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml;
import
org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration;
import org.apache.hadoop.ozone.container.common.utils.DatanodeStoreCache;
import org.apache.hadoop.ozone.container.common.utils.HddsVolumeUtil;
import org.apache.hadoop.ozone.container.common.utils.RawDB;
import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil;
-import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
-import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
-import
org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil;
import org.apache.hadoop.ozone.container.upgrade.VersionedDatanodeFeatures;
import
org.apache.hadoop.ozone.container.upgrade.VersionedDatanodeFeatures.SchemaV3;
import org.slf4j.Logger;
@@ -240,62 +233,13 @@ public class HddsVolume extends StorageVolume {
}
for (File containerDir: containerDirs) {
- // Check the case where we have Schema V3 and
- // removing the container's entries from RocksDB fails.
// --------------------------------------------
// On datanode restart, we populate the container set
// based on the available datanode volumes and
// populate the container metadata based on the values in RocksDB.
// The container is in the tmp directory,
// so it won't be loaded in the container set
- // but there will be orphaned entries in the volume's RocksDB.
// --------------------------------------------
- // For every .container file we find under /tmp,
- // we use it to get the RocksDB entries and delete them.
- // If the .container file doesn't exist then the contents of the
- // directory are probably leftovers of a failed delete and
- // the RocksDB entries must have already been removed.
- // In any case we can proceed with deleting the directory's contents.
- // --------------------------------------------
- // Get container file and check Schema version. If Schema is V3
- // then remove the container from RocksDB.
- File containerFile = ContainerUtils.getContainerFile(containerDir);
-
- if (containerFile.exists()) {
- KeyValueContainerData keyValueContainerData;
- try {
- ContainerData containerData =
- ContainerDataYaml.readContainerFile(containerFile);
- keyValueContainerData = (KeyValueContainerData) containerData;
- } catch (IOException ex) {
- LOG.warn("Failed to read container file {}. Container cannot be " +
- "removed from the delete directory.", containerFile, ex);
- continue;
- }
-
- if (keyValueContainerData.hasSchema(OzoneConsts.SCHEMA_V3)) {
- // Container file doesn't include the volume
- // so we need to set it here in order to get the DB location.
- keyValueContainerData.setVolume(this);
- File dbFile = KeyValueContainerLocationUtil
- .getContainerDBFile(keyValueContainerData);
- keyValueContainerData.setDbFile(dbFile);
- try {
- // Remove container from Rocks DB
- BlockUtils.removeContainerFromDB(keyValueContainerData, getConf());
- } catch (IOException ex) {
- LOG.warn("Failed to remove container data from DB while" +
- "deleting container {}. Container cannot be removed from" +
- "the delete directory {}.",
- keyValueContainerData.getContainerID(),
- deletedContainerDir, ex);
- continue;
- }
- }
- }
-
- // If the container file was already deleted, the RocksDB entries were
- // cleared.
try {
if (containerDir.isDirectory()) {
FileUtils.deleteDirectory(containerDir);
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 7d5ab92e81..d6d0e74ce0 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
@@ -306,7 +306,10 @@ public class KeyValueContainer implements
Container<KeyValueContainerData> {
public void delete() throws StorageContainerException {
long containerId = containerData.getContainerID();
try {
- KeyValueContainerUtil.removeContainer(containerData, config);
+ // Delete the Container from tmp directory.
+ File tmpDirectoryPath = KeyValueContainerUtil.getTmpDirectoryPath(
+ containerData, containerData.getVolume()).toFile();
+ FileUtils.deleteDirectory(tmpDirectoryPath);
} catch (StorageContainerException ex) {
// Disk needs replacement.
throw ex;
@@ -365,6 +368,26 @@ public class KeyValueContainer implements
Container<KeyValueContainerData> {
prevState);
}
+ @Override
+ public void markContainerForDelete() {
+ writeLock();
+ ContainerDataProto.State prevState = containerData.getState();
+ try {
+ containerData.setState(ContainerDataProto.State.DELETED);
+ File containerFile = getContainerFile();
+ // update the new container data to .container File
+ updateContainerFile(containerFile);
+ } catch (IOException ioe) {
+ LOG.error("Exception occur while update container {} state",
+ containerData.getContainerID(), ioe);
+ } finally {
+ writeUnlock();
+ }
+ LOG.info("Moving container {} to state {} from state:{}",
+ containerData.getContainerPath(), containerData.getState(),
+ prevState);
+ }
+
@Override
public void quasiClose() throws StorageContainerException {
closeAndFlushIfNeeded(containerData::quasiCloseContainer);
@@ -738,7 +761,7 @@ public class KeyValueContainer implements
Container<KeyValueContainerData> {
containerData.getContainerID());
}
- static File getContainerFile(String metadataPath, long containerId) {
+ public static File getContainerFile(String metadataPath, long containerId) {
return new File(metadataPath,
containerId + OzoneConsts.CONTAINER_EXTENSION);
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index 5db14e5d87..d43900a6c5 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -1348,10 +1348,18 @@ public class KeyValueHandler extends Handler {
(KeyValueContainerData) container.getContainerData();
HddsVolume hddsVolume = keyValueContainerData.getVolume();
- // Rename container location
+ // Steps to delete
+ // 1. container marked deleted
+ // 2. container is removed from container set
+ // 3. container db handler and content removed from db
+ // 4. container moved to tmp folder
+ // 5. container content deleted from tmp folder
try {
-
KeyValueContainerUtil.moveToDeletedContainerDir(keyValueContainerData,
- hddsVolume);
+ container.markContainerForDelete();
+ long containerId = container.getContainerData().getContainerID();
+ containerSet.removeContainer(containerId);
+ ContainerLogger.logDeleted(container.getContainerData(), force);
+ KeyValueContainerUtil.removeContainer(keyValueContainerData, conf);
} catch (IOException ioe) {
LOG.error("Failed to move container under " + hddsVolume
.getDeletedContainerDir());
@@ -1361,18 +1369,7 @@ public class KeyValueHandler extends Handler {
triggerVolumeScanAndThrowException(container, errorMsg,
CONTAINER_INTERNAL_ERROR);
}
-
- if (LOG.isDebugEnabled()) {
- String containerPath = keyValueContainerData
- .getContainerPath();
- File containerDir = new File(containerPath);
-
- LOG.debug("Container {} has been successfully moved under {}",
- containerDir.getName(), hddsVolume.getDeletedContainerDir());
- }
}
- long containerId = container.getContainerData().getContainerID();
- containerSet.removeContainer(containerId);
} catch (StorageContainerException e) {
throw e;
} catch (IOException e) {
@@ -1390,8 +1387,6 @@ public class KeyValueHandler extends Handler {
}
// Avoid holding write locks for disk operations
container.delete();
- container.getContainerData().setState(State.DELETED);
- ContainerLogger.logDeleted(container.getContainerData(), force);
sendICR(container);
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
index 1b714dacbf..f47d17d738 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
@@ -49,7 +49,6 @@ import
org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaTwoImpl;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V1;
/**
@@ -130,25 +129,30 @@ public final class KeyValueContainerUtil {
}
/**
- * remove Container if it is empty.
- * <p>
- * There are three things we need to delete.
- * <p>
- * 1. Container file and metadata file. 2. The Level DB file 3. The path that
- * we created on the data location.
+ * remove Container 1. remove db, 2. move to tmp directory.
*
* @param containerData - Data of the container to remove.
- * @param conf - configuration of the cluster.
* @throws IOException
*/
- public static void removeContainer(KeyValueContainerData containerData,
- ConfigurationSource conf)
+ public static void removeContainer(
+ KeyValueContainerData containerData, ConfigurationSource conf)
throws IOException {
Preconditions.checkNotNull(containerData);
- File containerMetaDataPath = new File(containerData
- .getMetadataPath());
- File chunksPath = new File(containerData.getChunksPath());
+ KeyValueContainerUtil.removeContainerDB(containerData, conf);
+ KeyValueContainerUtil.moveToDeletedContainerDir(containerData,
+ containerData.getVolume());
+ }
+ /**
+ * remove Container db, the Level DB file.
+ *
+ * @param containerData - Data of the container to remove.
+ * @param conf - configuration of the cluster.
+ * @throws IOException
+ */
+ public static void removeContainerDB(
+ KeyValueContainerData containerData, ConfigurationSource conf)
+ throws IOException {
if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) {
// DB failure is catastrophic, the disk needs to be replaced.
// In case of an exception, LOG the message and rethrow the exception.
@@ -163,15 +167,6 @@ public final class KeyValueContainerUtil {
// Close the DB connection and remove the DB handler from cache
BlockUtils.removeDB(containerData, conf);
}
-
- // Delete the Container MetaData path.
- FileUtils.deleteDirectory(containerMetaDataPath);
-
- //Delete the Container Chunks Path.
- FileUtils.deleteDirectory(chunksPath);
-
- //Delete Container directory
- FileUtils.deleteDirectory(containerMetaDataPath.getParentFile());
}
/**
@@ -472,15 +467,18 @@ public final class KeyValueContainerUtil {
* on the default container path on the disk.
*
* Delete operation for Schema < V3
- * 1. Container directory renamed to tmp directory.
- * 2. Container is removed from in memory container set.
- * 3. Container is deleted from tmp directory.
+ * 1. Container is marked DELETED
+ * 2. Container is removed from memory container set
+ * 3. Container DB handler from cache is removed and closed
+ * 4. Container directory renamed to tmp directory.
+ * 5. Container is deleted from tmp directory.
*
* Delete operation for Schema V3
- * 1. Container directory renamed to tmp directory.
- * 2. Container is removed from in memory container set.
- * 3. Container's entries are removed from RocksDB.
- * 4. Container is deleted from tmp directory.
+ * 1. Container is marked DELETED
+ * 2. Container is removed from memory container set
+ * 3. Container from DB is removed
+ * 4. Container directory renamed to tmp directory.
+ * 5. Container is deleted from tmp directory.
*
* @param keyValueContainerData
* @return true if renaming was successful
@@ -490,10 +488,8 @@ public final class KeyValueContainerUtil {
HddsVolume hddsVolume) throws IOException {
String containerPath = keyValueContainerData.getContainerPath();
File container = new File(containerPath);
- String containerDirName = container.getName();
-
- Path destinationDirPath = hddsVolume.getDeletedContainerDir().toPath()
- .resolve(Paths.get(containerDirName));
+ Path destinationDirPath = getTmpDirectoryPath(keyValueContainerData,
+ hddsVolume);
File destinationDirFile = destinationDirPath.toFile();
// If a container by the same name was moved to the delete directory but
@@ -504,14 +500,18 @@ public final class KeyValueContainerUtil {
}
Files.move(container.toPath(), destinationDirPath);
+ LOG.debug("Container {} has been successfully moved under {}",
+ container.getName(), hddsVolume.getDeletedContainerDir());
+ }
- // Updating in memory values of the container's location
- // which is used by KeyValueContainerUtil#removeContainer().
- // In case of a datanode restart these values won't persist but
- // tmp delete directory will be wiped, so this won't be an issue.
- keyValueContainerData.setMetadataPath(destinationDirPath +
- OZONE_URI_DELIMITER + OzoneConsts.CONTAINER_META_PATH);
- keyValueContainerData.setChunksPath(destinationDirPath +
- OZONE_URI_DELIMITER + OzoneConsts.STORAGE_DIR_CHUNKS);
+ public static Path getTmpDirectoryPath(
+ KeyValueContainerData keyValueContainerData,
+ HddsVolume hddsVolume) {
+ String containerPath = keyValueContainerData.getContainerPath();
+ File container = new File(containerPath);
+ String containerDirName = container.getName();
+ Path destinationDirPath = hddsVolume.getDeletedContainerDir().toPath()
+ .resolve(Paths.get(containerDirName));
+ return destinationDirPath;
}
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java
index 573cee9bf5..1ffc9e00cb 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java
@@ -34,6 +34,8 @@ import
org.apache.hadoop.ozone.container.common.volume.HddsVolume;
import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
+
+import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.DELETED;
import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
.ContainerDataProto.State.RECOVERING;
@@ -212,12 +214,17 @@ public class ContainerReader implements Runnable {
config);
if (kvContainer.getContainerState() == RECOVERING) {
if (shouldDeleteRecovering) {
+ cleanupContainer(hddsVolume, kvContainer);
kvContainer.delete();
LOG.info("Delete recovering container {}.",
kvContainer.getContainerData().getContainerID());
}
return;
}
+ if (kvContainer.getContainerState() == DELETED) {
+ cleanupContainer(hddsVolume, kvContainer);
+ return;
+ }
containerSet.addContainer(kvContainer);
} else {
throw new StorageContainerException("Container File is corrupted. " +
@@ -232,4 +239,21 @@ public class ContainerReader implements Runnable {
ContainerProtos.Result.UNKNOWN_CONTAINER_TYPE);
}
}
+
+ private void cleanupContainer(
+ HddsVolume volume, KeyValueContainer kvContainer) {
+ try {
+ LOG.info("Finishing delete of container {}.",
+ kvContainer.getContainerData().getContainerID());
+ // container information from db is removed for V3
+ // and container moved to tmp folder
+ // then container content removed from tmp folder
+ KeyValueContainerUtil.removeContainer(kvContainer.getContainerData(),
+ volume.getConf());
+ kvContainer.delete();
+ } catch (IOException ex) {
+ LOG.warn("Failed to remove deleted container {}.",
+ kvContainer.getContainerData().getContainerID(), ex);
+ }
+ }
}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsDatanodeService.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsDatanodeService.java
index 01188b6799..fa26ebcb62 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsDatanodeService.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsDatanodeService.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.ozone;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
@@ -37,6 +38,7 @@ import
org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
import org.apache.hadoop.ozone.container.keyvalue.ContainerTestVersionInfo;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
+import
org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerUtil;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.hadoop.util.ServicePlugin;
@@ -156,8 +158,9 @@ public class TestHddsDatanodeService {
KeyValueContainer container = ContainerTestUtils
.addContainerToDeletedDir(
hddsVolume, clusterId, conf, schemaVersion);
- assertTrue(container.getContainerFile().exists());
- assertTrue(container.getContainerDBFile().exists());
+ Path containerTmpPath = KeyValueContainerUtil.getTmpDirectoryPath(
+ container.getContainerData(), hddsVolume);
+ assertTrue(containerTmpPath.toFile().exists());
File[] deletedContainersAfterShutdown =
hddsVolume.getDeletedContainerDir().listFiles();
assertNotNull(deletedContainersAfterShutdown);
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/ContainerTestUtils.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/ContainerTestUtils.java
index 530a59c35f..5b2a1c0831 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/ContainerTestUtils.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/ContainerTestUtils.java
@@ -206,9 +206,33 @@ public final class ContainerTestUtils {
HddsVolume volume, String clusterId,
OzoneConfiguration conf, String schemaVersion)
throws IOException {
+ KeyValueContainer container = addContainerToVolumeDir(volume, clusterId,
+ conf, schemaVersion);
+
+ // For testing, we are moving the container
+ // under the tmp directory, in order to delete
+ // it from there, during datanode startup or shutdown
+ KeyValueContainerUtil
+ .moveToDeletedContainerDir(container.getContainerData(), volume);
+
+ return container;
+ }
+
+ public static KeyValueContainer addContainerToVolumeDir(
+ HddsVolume volume, String clusterId,
+ OzoneConfiguration conf, String schemaVersion)
+ throws IOException {
+ long containerId = ContainerTestHelper.getTestContainerID();
+ return addContainerToVolumeDir(volume, clusterId, conf, schemaVersion,
+ containerId);
+ }
+
+ public static KeyValueContainer addContainerToVolumeDir(
+ HddsVolume volume, String clusterId,
+ OzoneConfiguration conf, String schemaVersion, long containerId)
+ throws IOException {
VolumeChoosingPolicy volumeChoosingPolicy =
new RoundRobinVolumeChoosingPolicy();
- long containerId = ContainerTestHelper.getTestContainerID();
ContainerLayoutVersion layout = ContainerLayoutVersion.FILE_PER_BLOCK;
KeyValueContainerData keyValueContainerData = new KeyValueContainerData(
@@ -224,12 +248,6 @@ public final class ContainerTestUtils {
container.close();
- // For testing, we are moving the container
- // under the tmp directory, in order to delete
- // it from there, during datanode startup or shutdown
- KeyValueContainerUtil
- .moveToDeletedContainerDir(keyValueContainerData, volume);
-
return container;
}
}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
index 8be60132be..15b0b8fc05 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
@@ -289,6 +289,7 @@ public class TestContainerPersistence {
Assert.assertTrue(containerSet.getContainerMapCopy()
.containsKey(testContainerID));
+ KeyValueContainerUtil.removeContainer(container.getContainerData(), conf);
container.delete();
containerSet.removeContainer(testContainerID);
Assert.assertFalse(containerSet.getContainerMapCopy()
@@ -352,6 +353,9 @@ public class TestContainerPersistence {
// Block data and metadata tables should have data.
assertContainerInSchema3DB(containerData, blockID);
+ KeyValueContainerUtil.removeContainerDB(
+ container.getContainerData(),
+ container.getContainerData().getVolume().getConf());
container.delete();
containerSet.removeContainer(testContainerID);
Assert.assertFalse(containerSet.getContainerMapCopy()
@@ -475,13 +479,18 @@ public class TestContainerPersistence {
.collect(Collectors.toSet());
Assert.assertEquals(2, deleteDirFiles.size());
- File container1Dir = new File(container1Data.getContainerPath());
+ File container1Dir = KeyValueContainerUtil.getTmpDirectoryPath(
+ container1Data, hddsVolume).toFile();
Assert.assertTrue(deleteDirFiles.contains(container1Dir));
- File container2Dir = new File(container2Data.getContainerPath());
+ File container2Dir = KeyValueContainerUtil.getTmpDirectoryPath(
+ container2Data, hddsVolume).toFile();
Assert.assertTrue(deleteDirFiles.contains(container2Dir));
// Delete container1 from the disk. Container2 should remain in the
// deleted containers directory.
+ KeyValueContainerUtil.removeContainerDB(
+ container1.getContainerData(),
+ container1.getContainerData().getVolume().getConf());
container1.delete();
assertContainerNotInSchema3DB(container1Data, container1Block);
assertContainerInSchema3DB(container2Data, container2Block);
@@ -493,6 +502,9 @@ public class TestContainerPersistence {
Assert.assertEquals(deleteDirFilesArray[0], container2Dir);
// Delete container2 from the disk.
+ KeyValueContainerUtil.removeContainerDB(
+ container2.getContainerData(),
+ container2.getContainerData().getVolume().getConf());
container2.delete();
assertContainerNotInSchema3DB(container1Data, container1Block);
assertContainerNotInSchema3DB(container2Data, container2Block);
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 713936ed58..7efbe8a1b3 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
@@ -222,6 +222,8 @@ public class TestKeyValueContainer {
keyValueContainer.exportContainerData(fos, packer);
}
+ KeyValueContainerUtil.removeContainer(
+ keyValueContainer.getContainerData(), CONF);
keyValueContainer.delete();
// import container.
@@ -253,6 +255,8 @@ public class TestKeyValueContainer {
}
//delete the original one
+ KeyValueContainerUtil.removeContainer(
+ keyValueContainer.getContainerData(), CONF);
keyValueContainer.delete();
//create a new one
@@ -468,6 +472,8 @@ public class TestKeyValueContainer {
keyValueContainer = new KeyValueContainer(
keyValueContainerData, CONF);
keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId);
+ KeyValueContainerUtil.removeContainer(
+ keyValueContainer.getContainerData(), CONF);
keyValueContainer.delete();
String containerMetaDataPath = keyValueContainerData
@@ -807,6 +813,8 @@ public class TestKeyValueContainer {
}
//delete the original one
+ KeyValueContainerUtil.removeContainer(
+ keyValueContainer.getContainerData(), CONF);
keyValueContainer.delete();
//create a new one
@@ -852,6 +860,8 @@ public class TestKeyValueContainer {
}
//delete the original one
+ KeyValueContainerUtil.removeContainer(
+ keyValueContainer.getContainerData(), CONF);
keyValueContainer.delete();
//create a new one
KeyValueContainerData containerData =
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
index 31c9173bd8..745e8a0682 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
@@ -42,6 +42,7 @@ import
org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion;
import org.apache.hadoop.ozone.container.common.impl.ContainerSet;
import org.apache.hadoop.ozone.container.common.impl.HddsDispatcher;
+import org.apache.hadoop.ozone.container.common.interfaces.Container;
import org.apache.hadoop.ozone.container.common.interfaces.Handler;
import
org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
import org.apache.hadoop.ozone.container.common.statemachine.StateContext;
@@ -420,14 +421,14 @@ public class TestKeyValueHandler {
kvHandler.handleCreateContainer(createContainer2, null);
Assert.assertEquals(3, icrReceived.get());
- Assert.assertNotNull(containerSet.getContainer(container2ID));
+ Container<?> container = containerSet.getContainer(container2ID);
+ Assert.assertNotNull(container);
File deletedContainerDir = hddsVolume.getDeletedContainerDir();
// to simulate failed move
File dummyDir = new File(DUMMY_PATH);
hddsVolume.setDeletedContainerDir(dummyDir);
try {
- kvHandler.deleteContainer(containerSet.getContainer(container2ID),
- true);
+ kvHandler.deleteContainer(container, true);
} catch (StorageContainerException sce) {
Assert.assertTrue(
sce.getMessage().contains("Failed to move container"));
@@ -440,7 +441,9 @@ public class TestKeyValueHandler {
hddsVolume.failVolume();
GenericTestUtils.LogCapturer kvHandlerLogs =
GenericTestUtils.LogCapturer.captureLogs(KeyValueHandler.getLogger());
- kvHandler.deleteContainer(containerSet.getContainer(container2ID), true);
+ // add the container back to containerSet as removed in previous delete
+ containerSet.addContainer(container);
+ kvHandler.deleteContainer(container, true);
String expectedLog =
"Delete container issued on containerID 2 which is " +
"in a failed volume";
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
index 229e883ad8..e9f60b5c6c 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
@@ -42,11 +42,13 @@ import
org.apache.hadoop.ozone.container.keyvalue.ContainerTestVersionInfo;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
+import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -57,6 +59,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
+import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.DELETED;
import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING;
import static
org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded;
import static org.mockito.ArgumentMatchers.anyList;
@@ -379,4 +382,64 @@ public class TestContainerReader {
// opens and closed them avoiding the cache.
Assert.assertEquals(0, cache.size());
}
+
+ @Test
+ public void testMarkedDeletedContainerCleared() throws Exception {
+ KeyValueContainerData containerData = new KeyValueContainerData(
+ 101, layout, (long) StorageUnit.GB.toBytes(5),
+ UUID.randomUUID().toString(), datanodeId.toString());
+ //create a container with deleted state
+ containerData.setState(DELETED);
+
+ KeyValueContainer kvContainer =
+ new KeyValueContainer(containerData, conf);
+ kvContainer.create(
+ volumeSet, volumeChoosingPolicy, clusterId);
+ long baseCount = 0;
+ if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) {
+ // add db entry for the container ID 101 for V3
+ baseCount = addDbEntry(containerData);
+ }
+ ContainerReader containerReader = new ContainerReader(volumeSet,
+ hddsVolume, containerSet, conf, false);
+
+ containerReader.run();
+
+ // assert that tmp dir is empty
+ File[] leftoverContainers =
+ hddsVolume.getDeletedContainerDir().listFiles();
+ Assertions.assertNotNull(leftoverContainers);
+ Assertions.assertEquals(0, leftoverContainers.length);
+
+ Assertions.assertNull(containerSet.getContainer(101));
+
+ if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) {
+ // verify if newly added container is not present as added
+ try (DBHandle dbHandle = BlockUtils.getDB(
+ kvContainer.getContainerData(), conf)) {
+ DatanodeStoreSchemaThreeImpl store = (DatanodeStoreSchemaThreeImpl)
+ dbHandle.getStore();
+ Assertions.assertEquals(baseCount, store.getMetadataTable()
+ .getEstimatedKeyCount());
+ }
+ }
+ }
+
+ private long addDbEntry(KeyValueContainerData containerData)
+ throws Exception {
+ try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) {
+ DatanodeStoreSchemaThreeImpl store = (DatanodeStoreSchemaThreeImpl)
+ dbHandle.getStore();
+ Table<String, Long> metadataTable = store.getMetadataTable();
+ long baseSize = metadataTable.getEstimatedKeyCount();
+ metadataTable.put(containerData.getBytesUsedKey(), 0L);
+ metadataTable.put(containerData.getBlockCountKey(), 0L);
+ metadataTable.put(containerData.getPendingDeleteBlockCountKey(), 0L);
+
+ // The new keys should have been added in the MetadataTable
+ Assertions.assertEquals(baseSize + 3,
+ metadataTable.getEstimatedKeyCount());
+ return baseSize;
+ }
+ }
}
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/common/TestEndPoint.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/common/TestEndPoint.java
index 62f4f116e2..577bf9f34d 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/common/TestEndPoint.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/common/TestEndPoint.java
@@ -17,7 +17,9 @@
package org.apache.hadoop.ozone.container.common;
import java.io.File;
+import java.io.IOException;
import java.net.InetSocketAddress;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -44,11 +46,9 @@ import org.apache.hadoop.hdds.scm.HddsTestUtils;
import org.apache.hadoop.hdds.scm.VersionInfo;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager;
-import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.ipc.RPC;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.ozone.OzoneConsts;
-import org.apache.hadoop.ozone.container.common.interfaces.DBHandle;
import
org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
import
org.apache.hadoop.ozone.container.common.statemachine.EndpointStateMachine;
import org.apache.hadoop.ozone.container.common.statemachine.StateContext;
@@ -60,9 +60,7 @@ import
org.apache.hadoop.ozone.container.common.volume.HddsVolume;
import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
-import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
-import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
-import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl;
+import
org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerUtil;
import org.apache.hadoop.ozone.container.ozoneimpl.ContainerController;
import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
import
org.apache.hadoop.ozone.container.replication.ReplicationServer.ReplicationConfig;
@@ -171,9 +169,6 @@ public class TestEndPoint {
*/
@Test
public void testDeletedContainersClearedOnStartup() throws Exception {
- // TODO HDDS-8770. If cleanup of RocksDB is no longer required on tmp dir
- // cleanup, this test can be removed.
-
OzoneConfiguration conf = SCMTestUtils.getConf();
conf.setBoolean(OzoneConfigKeys.DFS_CONTAINER_IPC_RANDOM_PORT,
true);
@@ -182,70 +177,33 @@ public class TestEndPoint {
conf.setFromObject(new ReplicationConfig().setPort(0));
try (EndpointStateMachine rpcEndPoint = createEndpoint(conf,
serverAddress, 1000)) {
- DatanodeDetails datanodeDetails = randomDatanodeDetails();
- OzoneContainer ozoneContainer = new OzoneContainer(
- datanodeDetails, conf, getContext(datanodeDetails));
+ OzoneContainer ozoneContainer = createVolume(conf);
+ HddsVolume hddsVolume = (HddsVolume) ozoneContainer.getVolumeSet()
+ .getVolumesList().get(0);
+ KeyValueContainer kvContainer = addContainer(conf, hddsVolume);
+ // For testing, we are moving the container under the tmp directory,
+ // in order to delete during datanode startup or shutdown
+ KeyValueContainerUtil.moveToDeletedContainerDir(
+ kvContainer.getContainerData(), hddsVolume);
+ Path containerTmpPath = KeyValueContainerUtil.getTmpDirectoryPath(
+ kvContainer.getContainerData(), hddsVolume);
+ Assertions.assertTrue(containerTmpPath.toFile().exists());
+
rpcEndPoint.setState(EndpointStateMachine.EndPointStates.GETVERSION);
- String clusterId = scmServerImpl.getClusterId();
-
- MutableVolumeSet volumeSet = ozoneContainer.getVolumeSet();
- ContainerTestUtils.createDbInstancesForTestIfNeeded(volumeSet,
- clusterId, clusterId, conf);
- // VolumeSet for this test, contains only 1 volume
- Assertions.assertEquals(1, volumeSet.getVolumesList().size());
- StorageVolume volume = volumeSet.getVolumesList().get(0);
-
- // Check instanceof and typecast
- Assertions.assertTrue(volume instanceof HddsVolume);
- HddsVolume hddsVolume = (HddsVolume) volume;
-
- // Write some data before calling versionTask.call()
- // Create a container and move it under the tmp delete dir.
- KeyValueContainer container = ContainerTestUtils.
- addContainerToDeletedDir(hddsVolume, clusterId,
- conf, OzoneConsts.SCHEMA_V3);
- File containerDBFile = container.getContainerDBFile();
-
- Assertions.assertTrue(container.getContainerFile().exists());
- Assertions.assertTrue(containerDBFile.exists());
-
- KeyValueContainerData containerData = container.getContainerData();
-
- // Get DBHandle
- try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) {
-
- DatanodeStoreSchemaThreeImpl store = (DatanodeStoreSchemaThreeImpl)
- dbHandle.getStore();
- Table<String, Long> metadataTable = store.getMetadataTable();
-
- // DB MetadataTable is empty
- Assertions.assertEquals(0, metadataTable.getEstimatedKeyCount());
-
- // Add some keys to MetadataTable
- metadataTable.put(containerData.getBytesUsedKey(), 0L);
- metadataTable.put(containerData.getBlockCountKey(), 0L);
- metadataTable.put(containerData.getPendingDeleteBlockCountKey(), 0L);
-
- // The new keys should have been added in the MetadataTable
- Assertions.assertEquals(3, metadataTable.getEstimatedKeyCount());
-
- // versionTask.call() cleans the tmp dir and removes container from DB
- VersionEndpointTask versionTask = new VersionEndpointTask(rpcEndPoint,
- conf, ozoneContainer);
- EndpointStateMachine.EndPointStates newState = versionTask.call();
-
- Assertions.assertEquals(EndpointStateMachine.EndPointStates.REGISTER,
- newState);
-
- // assert that tmp dir is empty
- File[] leftoverContainers =
- hddsVolume.getDeletedContainerDir().listFiles();
- Assertions.assertNotNull(leftoverContainers);
- Assertions.assertEquals(0, leftoverContainers.length);
- // All DB keys have been deleted, MetadataTable should be empty
- Assertions.assertEquals(0, metadataTable.getEstimatedKeyCount());
- }
+ // versionTask.call() cleans the tmp dir and removes container from DB
+ VersionEndpointTask versionTask = new VersionEndpointTask(rpcEndPoint,
+ conf, ozoneContainer);
+ EndpointStateMachine.EndPointStates newState = versionTask.call();
+
+ Assertions.assertEquals(EndpointStateMachine.EndPointStates.REGISTER,
+ newState);
+
+ // assert that tmp dir is empty
+ File[] leftoverContainers =
+ hddsVolume.getDeletedContainerDir().listFiles();
+ Assertions.assertNotNull(leftoverContainers);
+ Assertions.assertEquals(0, leftoverContainers.length);
}
}
@@ -638,4 +596,36 @@ public class TestEndPoint {
return context;
}
+ private OzoneContainer createVolume(OzoneConfiguration conf)
+ throws IOException {
+ DatanodeDetails datanodeDetails = randomDatanodeDetails();
+ OzoneContainer ozoneContainer = new OzoneContainer(
+ datanodeDetails, conf, getContext(datanodeDetails));
+
+ String clusterId = scmServerImpl.getClusterId();
+
+ MutableVolumeSet volumeSet = ozoneContainer.getVolumeSet();
+ ContainerTestUtils.createDbInstancesForTestIfNeeded(volumeSet,
+ clusterId, clusterId, conf);
+ // VolumeSet for this test, contains only 1 volume
+ Assertions.assertEquals(1, volumeSet.getVolumesList().size());
+ StorageVolume volume = volumeSet.getVolumesList().get(0);
+
+ // Check instanceof and typecast
+ Assertions.assertTrue(volume instanceof HddsVolume);
+ return ozoneContainer;
+ }
+
+ private KeyValueContainer addContainer(
+ OzoneConfiguration conf, HddsVolume hddsVolume)
+ throws IOException {
+ String clusterId = scmServerImpl.getClusterId();
+ KeyValueContainer container = ContainerTestUtils.
+ addContainerToVolumeDir(hddsVolume, clusterId,
+ conf, OzoneConsts.SCHEMA_V3);
+ File containerDBFile = container.getContainerDBFile();
+ Assertions.assertTrue(container.getContainerFile().exists());
+ Assertions.assertTrue(containerDBFile.exists());
+ return container;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
