[
https://issues.apache.org/jira/browse/ACCUMULO-3577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17315853#comment-17315853
]
Christopher Tubbs edited comment on ACCUMULO-3577 at 4/6/21, 9:43 PM:
----------------------------------------------------------------------
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:patch}
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}
was (Author: ctubbsii):
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)
