You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2014/03/20 16:25:37 UTC
[20/20] git commit: ACCUMULO-2061 A few more minor fixes from
reviewboard.
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/8b8d565b
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/8b8d565b
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/8b8d565b
Branch: refs/heads/ACCUMULO-2061
Commit: 8b8d565b79a83ad6789356a18027cc2c0417b7f0
Parents: 45aec91
Author: Josh Elser <el...@apache.org>
Authored: Thu Mar 20 11:22:38 2014 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Thu Mar 20 11:22:38 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/8b8d565b/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/8b8d565b/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/8b8d565b/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/8b8d565b/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/8b8d565b/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/8b8d565b/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