kerneltime commented on code in PR #4367:
URL: https://github.com/apache/ozone/pull/4367#discussion_r1131537598


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java:
##########
@@ -1231,19 +1239,41 @@ private void deleteInternal(Container container, 
boolean force)
         }
         // Safety check that the container is empty.
         // If the container is not empty, it should not be deleted unless the
-        // container is beinf forcefully deleted (which happens when
+        // container is being forcefully deleted (which happens when
         // container is unhealthy or over-replicated).
         if (container.getContainerData().getBlockCount() != 0) {
+          metrics.incContainerDeleteFailedBlockCountNotZero();
           LOG.error("Received container deletion command for container {} but" 
+
-              " the container is not empty.",
-              container.getContainerData().getContainerID());
+              " the container is not empty with blockCount {}",
+              container.getContainerData().getContainerID(),
+              container.getContainerData().getBlockCount());
           throw new StorageContainerException("Non-force deletion of " +
               "non-empty container is not allowed.",
               DELETE_ON_NON_EMPTY_CONTAINER);
         }
+
+        if (checkIfNoBlockFiles && !container.isEmpty()) {
+          metrics.incContainerDeleteFailedNonEmpty();
+          LOG.error("Received container deletion command for container {} but" 
+
+                  " the container is not empty",
+              container.getContainerData().getContainerID());
+          throw new StorageContainerException("Non-force deletion of " +
+              "non-empty container:" +
+              container.getContainerData().getContainerID() +
+              " is not allowed.",
+              DELETE_ON_NON_EMPTY_CONTAINER);
+        }
+      } else {
+        metrics.incContainersForceDelete();
       }
       long containerId = container.getContainerData().getContainerID();
       containerSet.removeContainer(containerId);
+    } catch (IOException e) {
+      LOG.error("Could not determine if the container {} is empty",
+          container.getContainerData().getContainerID(), e);
+      throw new StorageContainerException("Could not determine if container "
+          + container.getContainerData().getContainerID() +
+          " is empty", DELETE_ON_NON_EMPTY_CONTAINER);

Review Comment:
   Checking the directory can lead to other IO exceptions. They all need to be 
mapped to the same `DELETE_ON_NON_EMPTY_CONTAINER` exception. Will make it 
explicit.



-- 
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