errose28 commented on code in PR #4995:
URL: https://github.com/apache/ozone/pull/4995#discussion_r1248284891


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java:
##########
@@ -87,20 +88,46 @@ public KeyValueContainerCheck(String metadataPath, 
ConfigurationSource conf,
    *
    * @return true : integrity checks pass, false : otherwise.
    */
-  public boolean fastCheck() {
+  public ScanResult fastCheck() {
     LOG.debug("Running basic checks for container {};", containerID);
-    boolean valid = false;
+
+    // Container directory should exist.
+    File containerDir = new File(metadataPath).getParentFile();
+    if (!containerDir.exists()) {
+      return ScanResult.unhealthy(ScanResult.FailureType.MISSING_CONTAINER_DIR,
+          containerDir, new FileNotFoundException("Container directory " +
+              containerDir + " not found."));
+    }
+
+    // Metadata directory should exist.
+    File metadataDir = new File(metadataPath);
+    if (!metadataDir.exists()) {
+      return ScanResult.unhealthy(ScanResult.FailureType.MISSING_METADATA_DIR,
+          metadataDir, new FileNotFoundException("Metadata directory " +
+              metadataDir + " not found."));
+    }
+
+    File containerFile = KeyValueContainer
+        .getContainerFile(metadataPath, containerID);
     try {
-      loadContainerData();
-      checkLayout();

Review Comment:
   I removed them for simplicity. I would have had to copy that method for 
metadata and chunks directories since each directory would need its own 
ScanResult code. Also each check may have needed its own unique scan result 
code.
   
   I don't think we really lost anything useful by simplifying the check. 
SecurityException is an unchecked exception and we don't handle it for any 
other file IO. We already check access to the contents of the directories when 
scanning the container file and the block files, so list check is redundant.
   
   Let me know if you think there is merit to doing additional checks on the 
directories other than just existence.



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