ctubbsii opened a new issue #2002: URL: https://github.com/apache/accumulo/issues/2002
**Describe the bug** Taken from https://issues.apache.org/jira/browse/ACCUMULO-3577 FileType doesn't parse out the volume from the directory correctly, because it incorrectly matches on partial path elements. For example, while trying to parse out the volume from a path that looks like: "file:///some/path/to/accumulo/tablesexample/tables", it will match on "file:///some/path/to/accumulo/" as the volume, and "tablesexample/tables" as the directory, instead of "file:///some/path/to/accumulo/tablesexample/" as the volume and "tables" as the directory. **Additional context** I [wrote a patch](https://issues.apache.org/jira/browse/ACCUMULO-3577?focusedCommentId=17315853&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17315853) and tried adding a few test cases to FileTypeTest as a sanity check and it seems to work: ```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/VolumeManager.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; ``` This code should also cover the case where there is no trailing slash because the directory we're looking for is the final path element. This patch needs to be submitted at as pull request, along with test cases added to FileTypeTest to ensure it works correctly. Some test cases I tried were: ``` assertEquals(new Path("tables"), FileType.TABLE.removeVolume(new Path("file:/a/accumulo/tables"))); assertEquals(new Path("tables/"), FileType.TABLE.removeVolume(new Path("file:/a/accumulo/tables/"))); assertEquals(new Path("file:/a/accumulo"), FileType.TABLE.getVolume(new Path("file:/a/accumulo/tables/"))); assertEquals(null, FileType.TABLE.getVolume(new Path("/a/accumulo/tables2/"))); assertEquals(new Path("tables/2b/t-001/C00.rf"), FileType.TABLE .removeVolume(new Path("file:/a/accumulo/tablesstuff/tables/2b/t-001/C00.rf"))); assertEquals(new Path("tables/tablestuff2"), FileType.TABLE.removeVolume(new Path("file:/a/accumulo/tablesstuff/tables/tablestuff2"))); ``` But, I just kind of threw these together, as a sanity check. I'm not sure if they're comprehensive enough. I think this could be wrapped up with minimal effort. This should also be applied to the 1.10 branch as well, because this could cause errors if the HDFS path names in the configured volumes cause this to incorrectly match. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
