ACCUMULO-2532 Use VolumeManager to check for tmpDir existence.
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/74ffff1a Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/74ffff1a Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/74ffff1a Branch: refs/heads/master Commit: 74ffff1a189c807995c33d400aca4fed7b183d38 Parents: 6420dd3 Author: Josh Elser <els...@apache.org> Authored: Tue Mar 25 15:56:03 2014 -0700 Committer: Josh Elser <els...@apache.org> Committed: Tue Mar 25 16:08:33 2014 -0700 ---------------------------------------------------------------------- .../apache/accumulo/server/util/FileUtil.java | 21 ++-- .../accumulo/server/util/FileUtilTest.java | 117 +++++++++++++++---- 2 files changed, 105 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/74ffff1a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java b/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java index fa186aa..4fee83d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java @@ -45,6 +45,7 @@ import org.apache.accumulo.core.iterators.SortedKeyValueIterator; import org.apache.accumulo.core.iterators.system.MultiIterator; import org.apache.accumulo.core.util.CachedConfiguration; import org.apache.accumulo.core.util.LocalityGroupUtil; +import org.apache.accumulo.core.volume.Volume; import org.apache.accumulo.server.ServerConstants; import org.apache.accumulo.server.fs.FileRef; import org.apache.accumulo.server.fs.VolumeManager; @@ -359,7 +360,7 @@ public class FileUtil { cleanupIndexOp(acuConf, tmpDir, fs, readers); } } - + protected static void cleanupIndexOp(AccumuloConfiguration acuConf, Path tmpDir, VolumeManager fs, ArrayList<FileSKVIterator> readers) throws IOException { // close all of the index sequence files for (FileSKVIterator r : readers) { @@ -371,20 +372,18 @@ public class FileUtil { log.error(e, e); } } - + if (tmpDir != null) { - // TODO ACCUMULO-2532 instance.dfs.dir is not required to be a part of - // the path for tmpDir. Could result in tmpDir not being removed when it should - // Needs unit tests - @SuppressWarnings("deprecation") - String tmpPrefix = acuConf.get(Property.INSTANCE_DFS_DIR) + "/tmp"; - if (tmpDir.toUri().getPath().startsWith(tmpPrefix)) + Volume v = fs.getVolumeByPath(tmpDir); + if (v.getFileSystem().exists(tmpDir)) { fs.deleteRecursively(tmpDir); - else - log.error("Did not delete tmp dir because it wasn't a tmp dir " + tmpDir); + return; + } + + log.error("Did not delete tmp dir because it wasn't a tmp dir " + tmpDir); } } - + private static long countIndexEntries(AccumuloConfiguration acuConf, Text prevEndRow, Text endRow, Collection<String> mapFiles, boolean useIndex, Configuration conf, VolumeManager fs, ArrayList<FileSKVIterator> readers) throws IOException { http://git-wip-us.apache.org/repos/asf/accumulo/blob/74ffff1a/server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java ---------------------------------------------------------------------- diff --git a/server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java b/server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java index 546b4ee..86678e1 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java @@ -44,21 +44,21 @@ public class FileUtilTest { @Test public void testCleanupIndexOpWithDfsDir() throws IOException { File dfsDir = Files.createTempDir(); - + try { // And a "unique" tmp directory for each volume File tmp1 = new File(dfsDir, "tmp"); tmp1.mkdirs(); Path tmpPath1 = new Path(tmp1.toURI()); - + HashMap<Property,String> testProps = new HashMap<Property,String>(); testProps.put(Property.INSTANCE_DFS_DIR, dfsDir.getAbsolutePath()); - + AccumuloConfiguration testConf = new FileUtilTestConfiguration(testProps); VolumeManager fs = VolumeManagerImpl.getLocal(dfsDir.getAbsolutePath()); - + FileUtil.cleanupIndexOp(testConf, tmpPath1, fs, new ArrayList<FileSKVIterator>()); - + Assert.assertFalse("Expected " + tmp1 + " to be cleaned up but it wasn't", tmp1.exists()); } finally { FileUtils.deleteQuietly(dfsDir); @@ -68,7 +68,7 @@ public class FileUtilTest { @Test public void testCleanupIndexOpWithCommonParentVolume() throws IOException { File accumuloDir = Files.createTempDir(); - + try { File volumeDir = new File(accumuloDir, "volumes"); volumeDir.mkdirs(); @@ -83,19 +83,57 @@ public class FileUtilTest { tmp1.mkdirs(); tmp2.mkdirs(); Path tmpPath1 = new Path(tmp1.toURI()), tmpPath2 = new Path(tmp2.toURI()); - + HashMap<Property,String> testProps = new HashMap<Property,String>(); testProps.put(Property.INSTANCE_VOLUMES, v1.toURI().toString() + "," + v2.toURI().toString()); - + AccumuloConfiguration testConf = new FileUtilTestConfiguration(testProps); VolumeManager fs = VolumeManagerImpl.getLocal(accumuloDir.getAbsolutePath()); - + FileUtil.cleanupIndexOp(testConf, tmpPath1, fs, new ArrayList<FileSKVIterator>()); - + Assert.assertFalse("Expected " + tmp1 + " to be cleaned up but it wasn't", tmp1.exists()); - + FileUtil.cleanupIndexOp(testConf, tmpPath2, fs, new ArrayList<FileSKVIterator>()); - + + Assert.assertFalse("Expected " + tmp2 + " to be cleaned up but it wasn't", tmp2.exists()); + } finally { + FileUtils.deleteQuietly(accumuloDir); + } + } + + @Test + public void testCleanupIndexOpWithCommonParentVolumeWithDepth() throws IOException { + File accumuloDir = Files.createTempDir(); + + try { + File volumeDir = new File(accumuloDir, "volumes"); + volumeDir.mkdirs(); + + // Make some directories to simulate multiple volumes + File v1 = new File(volumeDir, "v1"), v2 = new File(volumeDir, "v2"); + v1.mkdirs(); + v2.mkdirs(); + + // And a "unique" tmp directory for each volume + // Make sure we can handle nested directories (a single tmpdir with potentially multiple unique dirs) + File tmp1 = new File(new File(v1, "tmp"), "tmp_1"), tmp2 = new File(new File(v2, "tmp"), "tmp_1"); + tmp1.mkdirs(); + tmp2.mkdirs(); + Path tmpPath1 = new Path(tmp1.toURI()), tmpPath2 = new Path(tmp2.toURI()); + + HashMap<Property,String> testProps = new HashMap<Property,String>(); + testProps.put(Property.INSTANCE_VOLUMES, v1.toURI().toString() + "," + v2.toURI().toString()); + + AccumuloConfiguration testConf = new FileUtilTestConfiguration(testProps); + VolumeManager fs = VolumeManagerImpl.getLocal(accumuloDir.getAbsolutePath()); + + FileUtil.cleanupIndexOp(testConf, tmpPath1, fs, new ArrayList<FileSKVIterator>()); + + Assert.assertFalse("Expected " + tmp1 + " to be cleaned up but it wasn't", tmp1.exists()); + + FileUtil.cleanupIndexOp(testConf, tmpPath2, fs, new ArrayList<FileSKVIterator>()); + Assert.assertFalse("Expected " + tmp2 + " to be cleaned up but it wasn't", tmp2.exists()); } finally { FileUtils.deleteQuietly(accumuloDir); @@ -105,7 +143,7 @@ public class FileUtilTest { @Test public void testCleanupIndexOpWithoutCommonParentVolume() throws IOException { File accumuloDir = Files.createTempDir(); - + try { // Make some directories to simulate multiple volumes File v1 = new File(accumuloDir, "v1"), v2 = new File(accumuloDir, "v2"); @@ -117,19 +155,54 @@ public class FileUtilTest { tmp1.mkdirs(); tmp2.mkdirs(); Path tmpPath1 = new Path(tmp1.toURI()), tmpPath2 = new Path(tmp2.toURI()); - + + HashMap<Property,String> testProps = new HashMap<Property,String>(); + testProps.put(Property.INSTANCE_VOLUMES, v1.toURI().toString() + "," + v2.toURI().toString()); + + AccumuloConfiguration testConf = new FileUtilTestConfiguration(testProps); + VolumeManager fs = VolumeManagerImpl.getLocal(accumuloDir.getAbsolutePath()); + + FileUtil.cleanupIndexOp(testConf, tmpPath1, fs, new ArrayList<FileSKVIterator>()); + + Assert.assertFalse("Expected " + tmp1 + " to be cleaned up but it wasn't", tmp1.exists()); + + FileUtil.cleanupIndexOp(testConf, tmpPath2, fs, new ArrayList<FileSKVIterator>()); + + Assert.assertFalse("Expected " + tmp2 + " to be cleaned up but it wasn't", tmp2.exists()); + } finally { + FileUtils.deleteQuietly(accumuloDir); + } + } + + @Test + public void testCleanupIndexOpWithoutCommonParentVolumeWithDepth() throws IOException { + File accumuloDir = Files.createTempDir(); + + try { + // Make some directories to simulate multiple volumes + File v1 = new File(accumuloDir, "v1"), v2 = new File(accumuloDir, "v2"); + v1.mkdirs(); + v2.mkdirs(); + + // And a "unique" tmp directory for each volume + // Make sure we can handle nested directories (a single tmpdir with potentially multiple unique dirs) + File tmp1 = new File(new File(v1, "tmp"), "tmp_1"), tmp2 = new File(new File(v2, "tmp"), "tmp_1"); + tmp1.mkdirs(); + tmp2.mkdirs(); + Path tmpPath1 = new Path(tmp1.toURI()), tmpPath2 = new Path(tmp2.toURI()); + HashMap<Property,String> testProps = new HashMap<Property,String>(); testProps.put(Property.INSTANCE_VOLUMES, v1.toURI().toString() + "," + v2.toURI().toString()); - + AccumuloConfiguration testConf = new FileUtilTestConfiguration(testProps); VolumeManager fs = VolumeManagerImpl.getLocal(accumuloDir.getAbsolutePath()); - + FileUtil.cleanupIndexOp(testConf, tmpPath1, fs, new ArrayList<FileSKVIterator>()); - + Assert.assertFalse("Expected " + tmp1 + " to be cleaned up but it wasn't", tmp1.exists()); - + FileUtil.cleanupIndexOp(testConf, tmpPath2, fs, new ArrayList<FileSKVIterator>()); - + Assert.assertFalse("Expected " + tmp2 + " to be cleaned up but it wasn't", tmp2.exists()); } finally { FileUtils.deleteQuietly(accumuloDir); @@ -139,11 +212,11 @@ public class FileUtilTest { private static class FileUtilTestConfiguration extends AccumuloConfiguration { private DefaultConfiguration defaultConf = new DefaultConfiguration(); private Map<Property,String> properties; - + public FileUtilTestConfiguration(Map<Property,String> properties) { this.properties = properties; } - + @Override public String get(Property property) { String value = properties.get(property); @@ -157,6 +230,6 @@ public class FileUtilTest { public void getProperties(Map<String,String> props, PropertyFilter filter) { throw new UnsupportedOperationException(); } - + } }
