jojochuang commented on code in PR #8245:
URL: https://github.com/apache/ozone/pull/8245#discussion_r2078255632


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java:
##########
@@ -114,10 +114,17 @@ public static void createHardLinks(Path dbPath) throws 
IOException {
 
         // Create a link for each line.
         for (String l : lines) {
-          String from = l.split("\t")[1];
-          String to = l.split("\t")[0];
+          String[] parts = l.split("\t");

Review Comment:
   is there test code added for the new change in createHardLinks?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis_snapshot/OmRatisSnapshotProvider.java:
##########
@@ -158,14 +158,17 @@ public void downloadSnapshot(String leaderNodeID, File 
targetFile)
       connection.connect();
       int errorCode = connection.getResponseCode();
       if ((errorCode != HTTP_OK) && (errorCode != HTTP_CREATED)) {
-        throw new IOException("Unexpected exception when trying to reach " +
+        String message = "Unexpected exception when trying to reach " +
             "OM to download latest checkpoint. Checkpoint URL: " +
-            omCheckpointUrl + ". ErrorCode: " + errorCode);
+                omCheckpointUrl + ". ErrorCode: " + errorCode;
+        LOG.error(message);
+        throw new IOException(message);

Review Comment:
   1. connection object should call disconnect() before throwing the exception.
   2. This is unrelated, but the caller of this method, 
RDBSnapProvider.downloadDBSnapshotFromLeader() does not retry on exception.  
Should an exception be retried?
   



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServlet.java:
##########
@@ -764,69 +745,6 @@ private Set<String> getFiles(Path path, int truncateLength,
     return fileSet;
   }
 
-  /**
-   * Confirm fabricated link lines in hardlink file are properly
-   * formatted: "dir1/fabricatedFile dir2/fabricatedFile".
-   *
-   * The "fabricated" files/links are ones I've created by hand to
-   * fully test the code, (as opposed to the "natural" files/links
-   * created by the create snapshot process).
-   *
-   * @param directories Possible directories for the links to exist in.
-   * @param lines Text lines defining the link paths.
-   * @param testDirName Name of test directory.
-   */
-  private void checkFabricatedLines(Set<String> directories, List<String> 
lines,

Review Comment:
   removed because the usage is removed.



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