ACCUMULO-2061 Add comments, javadoc and other cleanup
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/550e5e61 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/550e5e61 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/550e5e61 Branch: refs/heads/master Commit: 550e5e61affc490f0931a8991d40f283b0282115 Parents: d45b723 Author: Josh Elser <els...@apache.org> Authored: Wed Mar 12 12:11:48 2014 -0400 Committer: Josh Elser <els...@apache.org> Committed: Thu Mar 20 18:58:53 2014 -0400 ---------------------------------------------------------------------- .../accumulo/core/file/rfile/PrintInfo.java | 9 +++++- .../org/apache/accumulo/core/volume/Volume.java | 15 ++++------ .../core/volume/VolumeConfiguration.java | 30 ++++++++++++-------- .../apache/accumulo/core/volume/VolumeImpl.java | 22 +++++++------- .../accumulo/server/fs/VolumeManager.java | 2 -- .../accumulo/server/fs/VolumeManagerImpl.java | 19 +++++++------ .../apache/accumulo/server/fs/VolumeUtil.java | 2 +- .../accumulo/server/util/ChangeSecret.java | 7 +++-- .../accumulo/server/fs/VolumeUtilTest.java | 2 +- 9 files changed, 59 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/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 4e39fc7..7c0f067 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 @@ -31,10 +31,12 @@ import org.apache.accumulo.core.volume.VolumeConfiguration; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.log4j.Logger; import com.beust.jcommander.Parameter; public class PrintInfo { + private static final Logger log = Logger.getLogger(PrintInfo.class); static class Opts extends Help { @Parameter(names = {"-d", "--dump"}, description = "dump the key/value pairs") @@ -50,6 +52,8 @@ public class PrintInfo { @SuppressWarnings("deprecation") AccumuloConfiguration aconf = AccumuloConfiguration.getSiteConfiguration(); + // TODO 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 FileSystem hadoopFs = VolumeConfiguration.getDefaultVolume(conf, aconf).getFileSystem(); FileSystem localFs = FileSystem.getLocal(conf); Opts opts = new Opts(); @@ -68,8 +72,11 @@ public class PrintInfo { FileSystem fs; if (arg.contains(":")) fs = path.getFileSystem(conf); - else + else { + // Recommend a URI is given for the above todo reason + log.warn("Attempting to find file across filesystems. Consider providing URI instead of path"); fs = hadoopFs.exists(path) ? hadoopFs : localFs; // fall back to local + } CachableBlockFile.Reader _rdr = new CachableBlockFile.Reader(fs, path, conf, null, null, aconf); Reader iter = new RFile.Reader(_rdr); http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/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 08f61d4..17b2bf3 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 @@ -21,39 +21,36 @@ import org.apache.hadoop.fs.Path; /** * Encapsulates a {@link FileSystem} and a base {@link Path} within that filesystem. This - * also avoid the necessity to pass around a Configuration. + * also avoid the necessity to pass around a Configuration. */ public interface Volume { /** * A {@link FileSystem} that Accumulo will use - * @return */ public FileSystem getFileSystem(); /** * The base path which Accumulo will use within the given {@link FileSystem} - * @return */ public String getBasePath(); - + /** * Convert the given Path into a Path that is relative to the base path for this Volume - * @param p - * @return + * @param p The suffix to use + * @return A Path for this Volume with the provided suffix */ public Path prefixChild(Path p); /** * Convert the given child path into a Path that is relative to the base path for this Volume - * @param p - * @return + * @param p The suffix to use + * @return A Path for this Volume with the provided suffix */ public Path prefixChild(String p); /** * Determine if the Path is valid on this Volume (contained by the basePath) - * @param p * @return True if path is contained within the basePath, false otherwise */ public boolean isValidPath(Path p); http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java b/core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java index 3005174..5db5bb2 100644 --- a/core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java @@ -30,10 +30,10 @@ import org.apache.hadoop.fs.Path; import com.google.common.base.Preconditions; public class VolumeConfiguration { - + public static Volume getVolume(String path, Configuration conf, AccumuloConfiguration acuconf) throws IOException { Preconditions.checkNotNull(path); - + if (path.contains(":")) { // An absolute path return create(new Path(path), conf); @@ -59,12 +59,15 @@ public class VolumeConfiguration { } } + /** + * @see org.apache.accumulo.core.volume.VolumeConfiguration#getVolumeUris(AccumuloConfiguration) + */ @Deprecated public static String getConfiguredBaseDir(AccumuloConfiguration conf) { String singleNamespace = conf.get(Property.INSTANCE_DFS_DIR); String dfsUri = conf.get(Property.INSTANCE_DFS_URI); String baseDir; - + if (dfsUri == null || dfsUri.isEmpty()) { Configuration hadoopConfig = CachedConfiguration.getInstance(); try { @@ -82,14 +85,15 @@ public class VolumeConfiguration { /** * Compute the URIs to be used by Accumulo + * * @param conf * @return */ public static String[] getVolumeUris(AccumuloConfiguration conf) { String ns = conf.get(Property.INSTANCE_VOLUMES); - + String configuredBaseDirs[]; - + if (ns == null || ns.isEmpty()) { // Fall back to using the old config values configuredBaseDirs = new String[] {getConfiguredBaseDir(conf)}; @@ -101,7 +105,7 @@ public class VolumeConfiguration { if (!namespace.contains(":")) { throw new IllegalArgumentException("Expected fully qualified URI for " + Property.INSTANCE_VOLUMES.getKey() + " got " + namespace); } - + try { // pass through URI to unescape hex encoded chars (e.g. convert %2C to "," char) configuredBaseDirs[i++] = new Path(new URI(namespace)).toString(); @@ -110,7 +114,7 @@ public class VolumeConfiguration { } } } - + return configuredBaseDirs; } @@ -126,24 +130,26 @@ public class VolumeConfiguration { /** * Create a Volume with the given FileSystem that writes to the default path - * @param fs A FileSystem to write to - * @return A Volume instance writing to the given FileSystem in the default path + * + * @param fs + * A FileSystem to write to + * @return A Volume instance writing to the given FileSystem in the default path */ @SuppressWarnings("deprecation") public static <T extends FileSystem> Volume create(T fs, AccumuloConfiguration acuconf) { String dfsDir = acuconf.get(Property.INSTANCE_DFS_DIR); return new VolumeImpl(fs, null == dfsDir ? Property.INSTANCE_DFS_DIR.getDefaultValue() : dfsDir); } - + public static <T extends FileSystem> Volume create(T fs, String basePath) { return new VolumeImpl(fs, basePath); } - + public static Volume create(String path, Configuration conf) throws IOException { Preconditions.checkNotNull(path); return create(new Path(path), conf); } - + public static Volume create(Path path, Configuration conf) throws IOException { return new VolumeImpl(path, conf); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/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 0aaf482..55ccfbc 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 @@ -24,30 +24,30 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; - /** - * + * Basic Volume implementation that contains a FileSystem and a base path + * that should be used within that filesystem. */ public class VolumeImpl implements Volume { protected final FileSystem fs; protected final String basePath; - + public VolumeImpl(Path path, Configuration conf) throws IOException { checkNotNull(path); checkNotNull(conf); - + this.fs = path.getFileSystem(conf); this.basePath = path.toUri().getPath(); } - + public VolumeImpl(FileSystem fs, String basePath) { checkNotNull(fs); checkNotNull(basePath); - + this.fs = fs; this.basePath = basePath; } - + @Override public FileSystem getFileSystem() { return fs; @@ -66,20 +66,20 @@ public class VolumeImpl implements Volume { @Override public boolean isValidPath(Path p) { checkNotNull(p); - + return p.toUri().getPath().startsWith(basePath); } - + @Override public boolean equals(Object o) { if (o instanceof VolumeImpl) { VolumeImpl other = (VolumeImpl) o; return getFileSystem().equals(other.getFileSystem()) && getBasePath().equals(other.getBasePath()); } - + return false; } - + @Override public String toString() { return getFileSystem() + " " + basePath; http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java ---------------------------------------------------------------------- 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 9b8fb98..cbfdb5e 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 @@ -160,13 +160,11 @@ public interface VolumeManager { /** * Fetch the default Volume - * @return */ public Volume getDefaultVolume(); /** * Fetch the configured Volumes, excluding the default Volume - * @return */ public Collection<Volume> getVolumes(); http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/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 ca5167d..b860f53 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 @@ -86,11 +86,12 @@ public class VolumeManagerImpl implements VolumeManager { inverted.put(volume.getFileSystem(), volume); } } - + public static org.apache.accumulo.server.fs.VolumeManager getLocal(String localBasePath) throws IOException { AccumuloConfiguration accConf = DefaultConfiguration.getDefaultConfiguration(); Volume defaultLocalVolume = VolumeConfiguration.create(FileSystem.getLocal(CachedConfiguration.getInstance()), localBasePath); - + + // The default volume gets placed in the map, but local filesystem is only used for testing purposes return new VolumeManagerImpl(Collections.singletonMap(DEFAULT, defaultLocalVolume), defaultLocalVolume, accConf); } @@ -112,16 +113,16 @@ public class VolumeManagerImpl implements VolumeManager { @Override public FSDataOutputStream create(Path path) throws IOException { checkNotNull(path); - + Volume v = getVolumeByPath(path); - + return v.getFileSystem().create(path); } @Override public FSDataOutputStream create(Path path, boolean overwrite) throws IOException { checkNotNull(path); - + Volume v = getVolumeByPath(path); return v.getFileSystem().create(path, overwrite); @@ -149,7 +150,7 @@ public class VolumeManagerImpl implements VolumeManager { Volume v = getVolumeByPath(path); FileSystem fs = v.getFileSystem(); - + if (bufferSize == 0) { fs.getConf().getInt("io.file.buffer.size", 4096); } @@ -314,7 +315,7 @@ public class VolumeManagerImpl implements VolumeManager { return defaultVolume; } - + log.debug("Could not determine volume for Path '" + path + "' from defined volumes"); } catch (IOException ex) { throw new RuntimeException(ex); @@ -404,7 +405,7 @@ public class VolumeManagerImpl implements VolumeManager { // The "default" Volume for Accumulo (in case no volumes are specified) for (String volumeUriOrDir : VolumeConfiguration.getVolumeUris(conf)) { if (volumeUriOrDir.equals(DEFAULT)) - // Cannot re-define the default volume + // Cannot re-define the default volume throw new IllegalArgumentException(); // We require a URI here, fail if it doesn't look like one @@ -554,7 +555,7 @@ public class VolumeManagerImpl implements VolumeManager { public Volume getDefaultVolume() { return defaultVolume; } - + @Override public Collection<Volume> getVolumes() { return volumesByName.values(); http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java index bcfb008..2ef438f 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java @@ -52,7 +52,7 @@ import org.apache.log4j.Logger; public class VolumeUtil { private static final Logger log = Logger.getLogger(VolumeUtil.class); - + private static boolean isActiveVolume(Path dir) { // consider relative path as active and take no action http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java b/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java index 3f33a0e..f0dcd14 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java @@ -146,9 +146,10 @@ public class ChangeSecret { private static void updateHdfs(VolumeManager fs, Instance inst, String newInstanceId) throws IOException { // Need to recreate the instanceId on all of them to keep consistency for (Volume v : fs.getVolumes()) { - v.getFileSystem().delete(ServerConstants.getInstanceIdLocation(v), true); - v.getFileSystem().mkdirs(ServerConstants.getInstanceIdLocation(v)); - v.getFileSystem().create(new Path(ServerConstants.getInstanceIdLocation(v), newInstanceId)).close(); + final Path instanceId = ServerConstants.getInstanceIdLocation(v); + v.getFileSystem().delete(instanceId, true); + v.getFileSystem().mkdirs(instanceId); + v.getFileSystem().create(new Path(instanceId, newInstanceId)).close(); } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/550e5e61/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java ---------------------------------------------------------------------- diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java index 3b905c9..0013d04 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java @@ -202,7 +202,7 @@ public class VolumeUtilTest { List<Pair<Path,Path>> replacements = new ArrayList<Pair<Path,Path>>(); replacements.add(new Pair<Path,Path>(new Path("file:/foo/v1"), new Path("file:/foo/v8"))); replacements.add(new Pair<Path,Path>(new Path("file:/foo/v2"), new Path("file:/foo/v9"))); - + FileType ft = FileType.TABLE; Assert.assertEquals("file:/foo/v8/tables/+r/root_tablet",
