accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [1/3] accumulo git commit: ACCUMULO-3400 Add sanity check for VolumeChooser return
Date Mon, 15 Dec 2014 19:45:51 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.6 b6f9a1e18 -> 86f7c1b08
  refs/heads/master dadb7bb11 -> b1dc56ec2


ACCUMULO-3400 Add sanity check for VolumeChooser return

  Backports the sanity check from ACCUMULO-3393 to 1.6. Ensures that
  VolumeChoosers only choose from the available options, and don't manifest
  choices of their own to return, which aren't available.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/86f7c1b0
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/86f7c1b0
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/86f7c1b0

Branch: refs/heads/1.6
Commit: 86f7c1b0866025171a1993abc01ce60ccfca859a
Parents: b6f9a1e
Author: Christopher Tubbs <ctubbsii@apache.org>
Authored: Mon Dec 15 14:33:51 2014 -0500
Committer: Christopher Tubbs <ctubbsii@apache.org>
Committed: Mon Dec 15 14:33:51 2014 -0500

----------------------------------------------------------------------
 .../accumulo/server/fs/VolumeManagerImpl.java   | 12 +++++++++-
 .../server/fs/VolumeManagerImplTest.java        | 23 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/86f7c1b0/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
index 38a8369..912d031 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
@@ -44,6 +44,7 @@ import org.apache.accumulo.core.volume.NonConfiguredVolume;
 import org.apache.accumulo.core.volume.Volume;
 import org.apache.accumulo.core.volume.VolumeConfiguration;
 import org.apache.accumulo.server.conf.ServerConfiguration;
+import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.NotImplementedException;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
@@ -571,9 +572,18 @@ public class VolumeManagerImpl implements VolumeManager {
     return getVolumeByPath(dir).getFileSystem().getContentSummary(dir);
   }
 
+  // Only used as a fall back if the configured chooser misbehaves.
+  private final VolumeChooser failsafeChooser = new RandomVolumeChooser();
+
   @Override
   public String choose(String[] options) {
-    return chooser.choose(options);
+    final String choice = chooser.choose(options);
+    if (!(ArrayUtils.contains(options, choice))) {
+      log.error("The configured volume chooser, '" +  chooser.getClass() + "', or one of
its delegates returned a volume not in the set of options provided; " +
+          "will continue by relying on a RandomVolumeChooser. You should investigate and
correct the named chooser.");
+      return failsafeChooser.choose(options);
+    }
+    return choice;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/86f7c1b0/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java
----------------------------------------------------------------------
diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java
b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java
index 582822a..26a23a3 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java
@@ -22,6 +22,7 @@ import java.util.List;
 import org.apache.accumulo.core.conf.ConfigurationCopy;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.server.fs.VolumeManager.FileType;
+import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.fs.Path;
 import org.junit.Assert;
 import org.junit.Before;
@@ -91,4 +92,26 @@ public class VolumeManagerImplTest {
     conf.set(Property.INSTANCE_VOLUMES, "viewfs://dummy");
     VolumeManagerImpl.get(conf);
   }
+
+  public static class WrongVolumeChooser implements VolumeChooser {
+    @Override
+    public String choose(String[] options) {
+      return "file://totally-not-given/";
+    }
+  }
+
+  @SuppressWarnings("deprecation")
+  private static final Property INSTANCE_DFS_URI = Property.INSTANCE_DFS_URI;
+
+  @Test
+  public void chooseFromOptions() throws Exception {
+    List<String> volumes = Arrays.asList("file://one/", "file://two/", "file://three/");
+    ConfigurationCopy conf = new ConfigurationCopy();
+    conf.set(INSTANCE_DFS_URI, volumes.get(0));
+    conf.set(Property.INSTANCE_VOLUMES, StringUtils.join(volumes,","));
+    conf.set(Property.GENERAL_VOLUME_CHOOSER, WrongVolumeChooser.class.getName());
+    VolumeManager vm = VolumeManagerImpl.get(conf);
+    String choice = vm.choose(volumes.toArray(new String[0]));
+    Assert.assertTrue("shouldn't see invalid options from misbehaving chooser.", volumes.contains(choice));
+  }
 }


Mime
View raw message