milleruntime commented on code in PR #2716:
URL: https://github.com/apache/accumulo/pull/2716#discussion_r875833212


##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java:
##########
@@ -51,64 +51,91 @@ public class GarbageCollectionAlgorithm {
 
   private static final Logger log = 
LoggerFactory.getLogger(GarbageCollectionAlgorithm.class);
 
+  /**
+   * This method takes a file or directory path and returns a relative path in 
1 of 2 forms:
+   *
+   * <pre>
+   *      1- For files: table-id/tablet-directory/filename.rf
+   *      2- For directories: table-id/tablet-directory
+   * </pre>
+   *
+   * For example, for full file path like 
hdfs://foo:6000/accumulo/tables/4/t0/F000.rf it will
+   * return 4/t0/F000.rf. For a directory that already is relative, like 4/t0, 
it will just return
+   * the original path. This method will also remove prefixed relative path.
+   */
   private String makeRelative(String path, int expectedLen) {
     String relPath = path;
 
+    // remove prefixed old relative path
     if (relPath.startsWith("../"))
       relPath = relPath.substring(3);
 
+    // remove trailing slash
     while (relPath.endsWith("/"))
       relPath = relPath.substring(0, relPath.length() - 1);
 
+    // remove beginning slash
     while (relPath.startsWith("/"))
       relPath = relPath.substring(1);
 
-    String[] tokens = relPath.split("/");
-
-    // handle paths like a//b///c
-    boolean containsEmpty = false;
-    for (String token : tokens) {
-      if (token.equals("")) {
-        containsEmpty = true;
-        break;
-      }
-    }
-
-    if (containsEmpty) {
-      ArrayList<String> tmp = new ArrayList<>();
-      for (String token : tokens) {
-        if (!token.equals("")) {
-          tmp.add(token);
-        }
-      }
-
-      tokens = tmp.toArray(new String[tmp.size()]);
-    }
+    String[] tokens = removeEmptyTokensFromPath(relPath);
 
     if (tokens.length > 3 && path.contains(":")) {
+      // full file path like hdfs://foo:6000/accumulo/tables/4/t0/F000.rf
       if (tokens[tokens.length - 4].equals(Constants.TABLE_DIR)
           && (expectedLen == 0 || expectedLen == 3)) {
+        // return the last 3 tokens after tables, like 4/t0/F000.rf
         relPath = tokens[tokens.length - 3] + "/" + tokens[tokens.length - 2] 
+ "/"
             + tokens[tokens.length - 1];
       } else if (tokens[tokens.length - 3].equals(Constants.TABLE_DIR)
           && (expectedLen == 0 || expectedLen == 2)) {
+        // return the last 2 tokens after tables, like 4/t0
         relPath = tokens[tokens.length - 2] + "/" + tokens[tokens.length - 1];
       } else {
-        throw new IllegalArgumentException(path);
+        throw new IllegalArgumentException("Failed to make path relative. Bad 
reference: " + path);
       }
     } else if (tokens.length == 3 && (expectedLen == 0 || expectedLen == 3)
         && !path.contains(":")) {
+      // we already have a relative path so return it, like 4/t0/F000.rf
       relPath = tokens[0] + "/" + tokens[1] + "/" + tokens[2];
     } else if (tokens.length == 2 && (expectedLen == 0 || expectedLen == 2)
         && !path.contains(":")) {
+      // return the last 2 tokens of the relative path, like 4/t0
       relPath = tokens[0] + "/" + tokens[1];
     } else {
-      throw new IllegalArgumentException(path);
+      throw new IllegalArgumentException("Failed to make path relative. Bad 
reference: " + path);
     }
 
     return relPath;
   }
 
+  /**
+   * Handle paths like a//b///c by dropping the empty tokens.
+   */
+  private String[] removeEmptyTokensFromPath(String relPath) {
+    String[] tokens = relPath.split("/");
+
+    boolean containsEmpty = false;
+    for (String token : tokens) {
+      if (token.equals("")) {
+        containsEmpty = true;
+        break;
+      }
+    }
+
+    if (containsEmpty) {

Review Comment:
   I dropped this method in favor of your other suggestion.



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

Reply via email to