ACCUMULO-2061 Don't store the defaultVolume in with the rest but expect it in the constructor.
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/c3214e0e Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/c3214e0e Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/c3214e0e Branch: refs/heads/ACCUMULO-2061 Commit: c3214e0e9433fc35cc822671ba66286e15d38f22 Parents: 3eb1593 Author: Josh Elser <els...@apache.org> Authored: Tue Mar 11 01:00:50 2014 -0400 Committer: Josh Elser <els...@apache.org> Committed: Tue Mar 11 01:00:50 2014 -0400 ---------------------------------------------------------------------- .../core/volume/VolumeConfiguration.java | 3 +++ .../accumulo/server/client/HdfsZooInstance.java | 1 + .../accumulo/server/fs/VolumeManager.java | 12 ++++++++++-- .../accumulo/server/fs/VolumeManagerImpl.java | 20 ++++++++++++-------- .../tserver/TabletServerSyncCheckTest.java | 2 +- .../java/org/apache/accumulo/test/VolumeIT.java | 20 +++++++++++--------- 6 files changed, 38 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/c3214e0e/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 3f4ab3e..3005174 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 @@ -46,6 +46,9 @@ public class VolumeConfiguration { public static Volume getDefaultVolume(Configuration conf, AccumuloConfiguration acuconf) throws IOException { @SuppressWarnings("deprecation") String uri = acuconf.get(Property.INSTANCE_DFS_URI); + + // By default pull from INSTANCE_DFS_URI, falling back to the Hadoop defined + // default filesystem (fs.defaultFS or the deprecated fs.default.name) if ("".equals(uri)) return create(FileSystem.get(conf), acuconf); else http://git-wip-us.apache.org/repos/asf/accumulo/blob/c3214e0e/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java b/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java index 8aa0be9..6bff9c5 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java @@ -135,6 +135,7 @@ public class HdfsZooInstance implements Instance { throw new RuntimeException(e); } Volume randVolume = fs.getVolumes().iterator().next(); + log.info("Looking for instanceId from " + randVolume); String instanceIdFromFile = ZooUtil.getInstanceIDFromHdfs(ServerConstants.getInstanceIdLocation(randVolume), acuConf); instanceId = instanceIdFromFile; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/c3214e0e/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 2f7cf42..c345ede 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 @@ -156,9 +156,17 @@ public interface VolumeManager { // decide on which of the given locations to create a new file String choose(String[] options); - + + /** + * 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/c3214e0e/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 a32b0bd..ca5167d 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 @@ -70,9 +70,9 @@ public class VolumeManagerImpl implements VolumeManager { AccumuloConfiguration conf; VolumeChooser chooser; - protected VolumeManagerImpl(Map<String,Volume> volumes, String defaultVolume, AccumuloConfiguration conf) { + protected VolumeManagerImpl(Map<String,Volume> volumes, Volume defaultVolume, AccumuloConfiguration conf) { this.volumesByName = volumes; - this.defaultVolume = volumes.get(defaultVolume); + this.defaultVolume = defaultVolume; // We may have multiple directories used in a single FileSystem (e.g. testing) this.volumesByFileSystem = HashMultimap.create(); invertVolumesByFileSystem(volumesByName, volumesByFileSystem); @@ -91,7 +91,7 @@ public class VolumeManagerImpl implements VolumeManager { AccumuloConfiguration accConf = DefaultConfiguration.getDefaultConfiguration(); Volume defaultLocalVolume = VolumeConfiguration.create(FileSystem.getLocal(CachedConfiguration.getInstance()), localBasePath); - return new VolumeManagerImpl(Collections.singletonMap("", defaultLocalVolume), "", accConf); + return new VolumeManagerImpl(Collections.singletonMap(DEFAULT, defaultLocalVolume), defaultLocalVolume, accConf); } @Override @@ -306,11 +306,16 @@ public class VolumeManagerImpl implements VolumeManager { return candidateVolume; } } - - throw new RuntimeException("Could not determine valid Volume for Path '" + path + "' from Volumes " + candidateVolumes); + + // For the same reason as we can have multiple Volumes within a single filesystem + // we could also not find a matching one. We should defer back to the defaultVolume + // e.g. volume rename with old path references + log.debug("Defaulting to " + defaultVolume + " as a valid volume could not be determined for " + path); + + return defaultVolume; } - log.info("Could not determine Volume for Path: " + path); + log.debug("Could not determine volume for Path '" + path + "' from defined volumes"); } catch (IOException ex) { throw new RuntimeException(ex); } @@ -397,7 +402,6 @@ public class VolumeManagerImpl implements VolumeManager { final Configuration hadoopConf = CachedConfiguration.getInstance(); // The "default" Volume for Accumulo (in case no volumes are specified) - volumes.put(DEFAULT, VolumeConfiguration.getDefaultVolume(hadoopConf, conf)); for (String volumeUriOrDir : VolumeConfiguration.getVolumeUris(conf)) { if (volumeUriOrDir.equals(DEFAULT)) // Cannot re-define the default volume @@ -411,7 +415,7 @@ public class VolumeManagerImpl implements VolumeManager { } } - return new VolumeManagerImpl(volumes, DEFAULT, conf); + return new VolumeManagerImpl(volumes, VolumeConfiguration.getDefaultVolume(hadoopConf, conf), conf); } @Override http://git-wip-us.apache.org/repos/asf/accumulo/blob/c3214e0e/server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java index a5a1deb..dad9a75 100644 --- a/server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java +++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java @@ -92,7 +92,7 @@ public class TabletServerSyncCheckTest { public TestVolumeManagerImpl(Map<String,Volume> volumes) { - super(volumes, volumes.keySet().iterator().next(), new ConfigurationCopy(Collections.<String,String> emptyMap())); + super(volumes, volumes.values().iterator().next(), new ConfigurationCopy(Collections.<String,String> emptyMap())); } @Override http://git-wip-us.apache.org/repos/asf/accumulo/blob/c3214e0e/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 a0efe45..7ac34f8 100644 --- a/test/src/test/java/org/apache/accumulo/test/VolumeIT.java +++ b/test/src/test/java/org/apache/accumulo/test/VolumeIT.java @@ -23,6 +23,7 @@ import java.io.BufferedOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -104,8 +105,9 @@ public class VolumeIT extends ConfigurableMacIT { @Override public void configure(MiniAccumuloConfigImpl cfg, Configuration hadoopCoreSite) { // Run MAC on two locations in the local file system - cfg.setProperty(Property.INSTANCE_DFS_URI, v1.toString()); - cfg.setProperty(Property.INSTANCE_DFS_DIR, "/accumulo"); + URI v1Uri = v1.toUri(); + cfg.setProperty(Property.INSTANCE_DFS_DIR, v1Uri.getPath()); + cfg.setProperty(Property.INSTANCE_DFS_URI, v1Uri.getScheme() + v1Uri.getHost()); cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," + v2.toString()); // use raw local file system so walogs sync and flush will work @@ -114,7 +116,7 @@ public class VolumeIT extends ConfigurableMacIT { super.configure(cfg, hadoopCoreSite); } - @Test + @Test(timeout = 2 * 60 * 1000) public void test() throws Exception { // create a table Connector connector = getConnector(); @@ -176,7 +178,7 @@ public class VolumeIT extends ConfigurableMacIT { Assert.assertEquals(expected, actual); } - @Test + @Test(timeout = 2 * 60 * 1000) public void testRelativePaths() throws Exception { List<String> expected = new ArrayList<String>(); @@ -390,7 +392,7 @@ public class VolumeIT extends ConfigurableMacIT { Assert.assertEquals(200, sum); } - @Test + @Test(timeout = 5 * 60 * 1000) public void testRemoveVolumes() throws Exception { String[] tableNames = getTableNames(2); @@ -447,12 +449,12 @@ public class VolumeIT extends ConfigurableMacIT { File v1f = new File(v1.toUri()); File v8f = new File(new File(v1.getParent().toUri()), "v8"); - v1f.renameTo(v8f); + Assert.assertTrue("Failed to rename " + v1f + " to " + v8f, v1f.renameTo(v8f)); Path v8 = new Path(v8f.toURI()); File v2f = new File(v2.toUri()); File v9f = new File(new File(v2.getParent().toUri()), "v9"); - v2f.renameTo(v9f); + Assert.assertTrue("Failed to rename " + v2f + " to " + v9f, v2f.renameTo(v9f)); Path v9 = new Path(v9f.toURI()); Configuration conf = new Configuration(false); @@ -494,12 +496,12 @@ public class VolumeIT extends ConfigurableMacIT { verifyVolumesUsed(tableNames[2], true, v8, v9); } - @Test + @Test(timeout = 5 * 60 * 1000) public void testCleanReplaceVolumes() throws Exception { testReplaceVolume(true); } - @Test + @Test(timeout = 5 * 60 * 1000) public void testDirtyReplaceVolumes() throws Exception { testReplaceVolume(false); }
