neils-dev commented on code in PR #3741:
URL: https://github.com/apache/ozone/pull/3741#discussion_r1092665501


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java:
##########
@@ -395,4 +413,158 @@ public static Path getMetadataDirectory(
     return Paths.get(metadataPath);
 
   }
+
+  /**
+   * Utilities for container_delete_service directory
+   * which is located under <volume>/hdds/<cluster-id>/tmp/.
+   * Containers will be moved under it before getting deleted
+   * to avoid, in case of failure, having artifact leftovers
+   * 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.
+   *
+   * 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.
+   */
+  public static class ContainerDeleteDirectory {
+
+    /**
+     * Delete all files under
+     * <volume>/hdds/<cluster-id>/tmp/container_delete_service.
+     */
+    public static synchronized void cleanTmpDir(HddsVolume hddsVolume)
+        throws IOException {
+      if (hddsVolume.getStorageState() != StorageVolume.VolumeState.NORMAL) {
+        LOG.debug("Call to clean tmp dir container_delete_service directory "
+                + "for {} while VolumeState {}",
+            hddsVolume.getStorageDir(),
+            hddsVolume.getStorageState().toString());
+        return;
+      }
+
+      // Initialize tmp and delete service directories
+      hddsVolume.checkTmpDirPaths(hddsVolume.getClusterID());
+
+      ListIterator<File> leftoversListIt = getDeleteLeftovers(hddsVolume);
+
+      while (leftoversListIt.hasNext()) {
+        File file = leftoversListIt.next();
+
+        // 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(file);
+        
+        ContainerData containerData = ContainerDataYaml
+            .readContainerFile(containerFile);
+        KeyValueContainerData keyValueContainerData =
+            (KeyValueContainerData) containerData;
+
+        if (keyValueContainerData.getSchemaVersion()
+            .equals(OzoneConsts.SCHEMA_V3)) {
+          // Container file doesn't include the volume
+          // so we need to set it here in order to get the db file.
+          keyValueContainerData.setVolume(hddsVolume);
+          File dbFile = KeyValueContainerLocationUtil
+              .getContainerDBFile(keyValueContainerData);
+          keyValueContainerData.setDbFile(dbFile);
+          // Remove container from Rocks DB
+          BlockUtils.removeContainerFromDB(keyValueContainerData,
+              hddsVolume.getConf());
+        }

Review Comment:
   The `removeContainerFromDB` can throw an exception should the delete from 
the volume DB fail.  We should catch this here, _log the error_, and continue 
iterating through the list of containers to delete.  On the next call for this 
volume, the failed container db entry will be retried to remove from the db and 
subsequently delete the container should it succeed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to