accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From els...@apache.org
Subject [19/25] git commit: ACCUMULO-2061 A few more minor fixes from reviewboard.
Date Fri, 21 Mar 2014 02:13:22 GMT
ACCUMULO-2061 A few more minor fixes from reviewboard.

Fixed a PrintInfo comment. Expand the VolumeImpl.isValidPath javadoc
and implementation to be more encompassing. Suppress warnings in VolumeIT.
Remove implementation of isValidPath from NonConfiguredVolume as it
could be misleading.


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

Branch: refs/heads/1.6.0-SNAPSHOT
Commit: 9172f52bc74de2590603c5282c19264447e33da6
Parents: 42f6b58
Author: Josh Elser <elserj@apache.org>
Authored: Thu Mar 20 11:22:38 2014 -0400
Committer: Josh Elser <elserj@apache.org>
Committed: Thu Mar 20 18:59:30 2014 -0400

----------------------------------------------------------------------
 .../accumulo/core/file/rfile/PrintInfo.java     |  7 +++--
 .../core/volume/NonConfiguredVolume.java        | 30 ++++++--------------
 .../org/apache/accumulo/core/volume/Volume.java |  4 +--
 .../apache/accumulo/core/volume/VolumeImpl.java | 16 ++++++++++-
 .../accumulo/server/fs/VolumeManagerImpl.java   |  8 +++---
 .../java/org/apache/accumulo/test/VolumeIT.java |  1 +
 6 files changed, 36 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java
index 7ed8f34..dc54b49 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java
@@ -52,8 +52,11 @@ public class PrintInfo {
 
     @SuppressWarnings("deprecation")
     AccumuloConfiguration aconf = AccumuloConfiguration.getSiteConfiguration();
-    // TODO ACCUMULO-2462 This will only work for RFiles in HDFS when the filesystem is defined
in the core-site.xml
-    // on the classpath if a path, and not a URI, is given
+    // TODO ACCUMULO-2462 This will only work for RFiles (path only, not URI) in HDFS when
the correct filesystem for the given file
+    // is on Property.INSTANCE_DFS_DIR or, when INSTANCE_DFS_DIR is not defined, is on the
default filesystem 
+    // defined in the Hadoop's core-site.xml
+    //
+    // A workaround is to always provide a URI to this class
     FileSystem hadoopFs = VolumeConfiguration.getDefaultVolume(conf, aconf).getFileSystem();
     FileSystem localFs  = FileSystem.getLocal(conf);
     Opts opts = new Opts();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java b/core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java
index 7dcbd88..3cfd1c2 100644
--- a/core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java
+++ b/core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java
@@ -16,25 +16,18 @@
  */
 package org.apache.accumulo.core.volume;
 
-import java.io.IOException;
-
 import org.apache.accumulo.core.conf.Property;
-import org.apache.accumulo.core.util.CachedConfiguration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.log4j.Logger;
 
 /**
- * Volume implementation which represents a Volume for which we have a FileSystem
- * but no base path because it is not configured via {@link Property#INSTANCE_VOLUMES}
+ * Volume implementation which represents a Volume for which we have a FileSystem but no
base path because it is not configured via
+ * {@link Property#INSTANCE_VOLUMES}
  * 
- * This is useful to handle volumes that have been removed from accumulo-site.xml but references
- * to these volumes have not been updated. This Volume should never be used to create new
files,
- * only to read existing files.
+ * This is useful to handle volumes that have been removed from accumulo-site.xml but references
to these volumes have not been updated. This Volume should
+ * never be used to create new files, only to read existing files.
  */
 public class NonConfiguredVolume implements Volume {
-  private static final Logger log = Logger.getLogger(NonConfiguredVolume.class);
-
   private FileSystem fs;
 
   public NonConfiguredVolume(FileSystem fs) {
@@ -48,27 +41,22 @@ public class NonConfiguredVolume implements Volume {
 
   @Override
   public String getBasePath() {
-    throw new UnsupportedOperationException("No base path known because this volume isn't
configured in accumulo-site.xml");
+    throw new UnsupportedOperationException("No base path known because this Volume isn't
configured in accumulo-site.xml");
   }
 
   @Override
   public Path prefixChild(Path p) {
-    throw new UnsupportedOperationException("Cannot prefix path because this volume isn't
configured in accumulo-site.xml");
+    throw new UnsupportedOperationException("Cannot prefix path because this Volume isn't
configured in accumulo-site.xml");
   }
 
   @Override
   public Path prefixChild(String p) {
-    throw new UnsupportedOperationException("Cannot prefix path because this volume isn't
configured in accumulo-site.xml");
+    throw new UnsupportedOperationException("Cannot prefix path because this Volume isn't
configured in accumulo-site.xml");
   }
 
   @Override
   public boolean isValidPath(Path p) {
-    try {
-      return fs.equals(p.getFileSystem(CachedConfiguration.getInstance()));
-    } catch (IOException e) {
-      log.debug("Cannot determine FileSystem from path: " + p, e);
-    }
-    return false;
+    throw new UnsupportedOperationException("Cannot determine if path is valid because this
Volume isn't configured in accumulo-site.xml");
   }
 
   @Override
@@ -89,4 +77,4 @@ public class NonConfiguredVolume implements Volume {
   public int hashCode() {
     return NonConfiguredVolume.class.hashCode() ^ this.fs.hashCode();
   }
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/core/src/main/java/org/apache/accumulo/core/volume/Volume.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/volume/Volume.java b/core/src/main/java/org/apache/accumulo/core/volume/Volume.java
index 17b2bf3..58b0ada 100644
--- a/core/src/main/java/org/apache/accumulo/core/volume/Volume.java
+++ b/core/src/main/java/org/apache/accumulo/core/volume/Volume.java
@@ -50,8 +50,8 @@ public interface Volume {
   public Path prefixChild(String p);
 
   /**
-   * Determine if the Path is valid on this Volume (contained by the basePath)
-   * @return True if path is contained within the basePath, false otherwise
+   * Determine if the Path is valid on this Volume. A Path is valid if it is contained
+   * in the Volume's FileSystem and is rooted beneath the basePath
    */
   public boolean isValidPath(Path p);
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java b/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java
index 55ccfbc..f902c35 100644
--- a/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java
@@ -20,15 +20,21 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.io.IOException;
 
+import jline.internal.Log;
+
+import org.apache.accumulo.core.util.CachedConfiguration;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.log4j.Logger;
 
 /**
  * Basic Volume implementation that contains a FileSystem and a base path 
  * that should be used within that filesystem.
  */
 public class VolumeImpl implements Volume {
+  private static final Logger log = Logger.getLogger(VolumeImpl.class);
+
   protected final FileSystem fs;
   protected final String basePath;
 
@@ -67,7 +73,15 @@ public class VolumeImpl implements Volume {
   public boolean isValidPath(Path p) {
     checkNotNull(p);
 
-    return p.toUri().getPath().startsWith(basePath);
+    try {
+      if (fs.equals(p.getFileSystem(CachedConfiguration.getInstance()))) {
+        return p.toUri().getPath().startsWith(basePath);
+      }
+    } catch (IOException e) {
+      log.warn("Could not determine filesystem from path: " + p);
+    }
+
+    return false;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/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 64a6390..6a3140b 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
@@ -314,12 +314,12 @@ public class VolumeManagerImpl implements VolumeManager {
           // we could also not find a matching one. We should still provide a Volume with
the
           // correct FileSystem even though we don't know what the proper base dir is
           // e.g. Files on volumes that are now removed
-          log.debug("Found no configured Volume for the given path: " + path);
-
-          return new NonConfiguredVolume(desiredFs);
+          log.debug("Found candidate Volumes for Path but none of the Paths are valid on
the candidates: " + path);
         }
 
-        log.debug("Could not determine volume for Path '" + path + "' from defined volumes");
+        log.debug("Could not determine volume for Path: " + path);
+
+        return new NonConfiguredVolume(desiredFs);
       } catch (IOException ex) {
         throw new RuntimeException(ex);
       }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9172f52b/test/src/test/java/org/apache/accumulo/test/VolumeIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/VolumeIT.java b/test/src/test/java/org/apache/accumulo/test/VolumeIT.java
index 6c1da2c..0a7ef3f 100644
--- a/test/src/test/java/org/apache/accumulo/test/VolumeIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/VolumeIT.java
@@ -102,6 +102,7 @@ public class VolumeIT extends ConfigurableMacIT {
     FileUtils.deleteQuietly(new File(v1.getParent().toUri()));
   }
 
+  @SuppressWarnings("deprecation")
   @Override
   public void configure(MiniAccumuloConfigImpl cfg, Configuration hadoopCoreSite) {
     // Run MAC on two locations in the local file system


Mime
View raw message