[jira] [Commented] (ACCUMULO-3577) VolumeManager FileType needs unit tests

["Christopher Tubbs (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ACCUMULO-3577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17315853#comment-17315853 ] 

Christopher Tubbs commented on ACCUMULO-3577:
---------------------------------------------

I thought that I had fixed this, but it looks like I had fixed a very similar bug in VolumeImpl and added test cases in VolumeImplTest. I was able to confirm that this is still a bug.

Here's a patch that seems to work, but needs to have test cases to cover it:

{code:diff}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManage
r.java
index 4e92ef5272..cbbdc467ea 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java
@@ -62,11 +62,16 @@ public interface VolumeManager extends AutoCloseable {
 
     private static int endOfVolumeIndex(String path, String dir) {
       // Strip off the suffix that starts with the FileType (e.g. tables, wal, etc)
-      int dirIndex = path.indexOf('/' + dir);
+      int dirIndex = path.indexOf('/' + dir + '/');
+
       if (dirIndex != -1) {
         return dirIndex;
       }
 
+      if (path.endsWith('/' + dir)) {
+        return path.length() - (dir.length() + 1);
+      }
+
       if (path.contains(":"))
         throw new IllegalArgumentException(path + " is absolute, but does not contain " + dir);
       return -1;
@@ -213,9 +218,8 @@ public interface VolumeManager extends AutoCloseable {
         log.error("multiple potential instances in {}", instanceDirectory);
         throw new RuntimeException(
             "Accumulo found multiple possible instance ids in " + instanceDirectory);
-      } else {
-        return files[0].getPath().getName();
       }
+      return files[0].getPath().getName();
     } catch (IOException e) {
       log.error("Problem reading instance id out of hdfs at " + instanceDirectory, e);
       throw new RuntimeException(
{code}

> VolumeManager FileType needs unit tests
> ---------------------------------------
>
>                 Key: ACCUMULO-3577
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-3577
>             Project: Accumulo
>          Issue Type: Bug
>            Reporter: Christopher Tubbs
>            Priority: Major
>
> The FileType enum in VolumeManager has some behavior which should be unit tested.
> In particular, it should check that the retrieval (or removal) of volumes from a path works correctly.
> At first glance, it looks to me like it is broken, because it is looking for the first occurrence of a directory which starts with {{/tables}}, in the case of tables. However, that will break if the volume itself had a path with {{/tables}} in it (example: {{hdfs://nn1/tablesorting-application/accumulo}}, which would have the directory {{hdfs://nn1/tablesorting-application/accumulo/tables/...}})



--
This message was sent by Atlassian Jira
(v8.3.4#803005)