You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by le...@apache.org on 2016/07/08 21:30:12 UTC
[05/21] hadoop git commit: Implement recursive delete for
LocalMetadataStore.
Implement recursive delete for LocalMetadataStore.
Recursive delete unit test now passes.
Also added test for recursive hash invalidation logic.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9d32cea2
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9d32cea2
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9d32cea2
Branch: refs/heads/s3_create
Commit: 9d32cea20738fac5d576466a2f4d63adcab76498
Parents: 198ed29
Author: Aaron Fabbri <fa...@cloudera.com>
Authored: Wed Jun 29 16:10:27 2016 -0700
Committer: Aaron Fabbri <fa...@cloudera.com>
Committed: Wed Jun 29 16:10:27 2016 -0700
----------------------------------------------------------------------
.../hadoop/fs/s3a/LocalMetadataStore.java | 47 +++++++++++++++++---
.../org/apache/hadoop/fs/s3a/MetadataStore.java | 6 +++
.../hadoop/fs/s3a/TestLocalMetadataStore.java | 21 +++++++++
3 files changed, 69 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9d32cea2/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/LocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/LocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/LocalMetadataStore.java
index 000ea89..94df47a 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/LocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/LocalMetadataStore.java
@@ -1,10 +1,12 @@
package org.apache.hadoop.fs.s3a;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.fs.Path;
import javax.annotation.Nullable;
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -14,6 +16,10 @@ import java.util.Map;
* The purpose of this is for unit testing. It could also be used to accelerate
* local-only operations where only one process is operating on a given object
* store.
+ * This MetadataStore does not enforce filesystem rules such as disallowing
+ * non-recursive removal of non-empty directories. It is assumed the caller
+ * already has to perform these sorts of checks.
+ *
* TODO make a singleton.
*/
public class LocalMetadataStore implements MetadataStore {
@@ -45,13 +51,21 @@ public class LocalMetadataStore implements MetadataStore {
// Delete entry from file cache, then from cached parent directory, if any
synchronized (this) {
+ // Remove target file/dir
CachedFileStatus cfs = fileHash.remove(f);
- // TODO XXX handle recursive option
- if (cfs != null) {
- dirHashDeleteFile(cfs.getFileStatus().getPath());
- return true;
+
+ if (cfs == null)
+ return false;
+
+ // Update parent dir listing, if any
+ dirHashDeleteFile(cfs.getFileStatus().getPath());
+
+ if (recursive && cfs.getFileStatus().isDirectory()) {
+ // Remove all entries that have this dir as path prefix.
+ clearHashByPrefix(f, dirHash);
+ clearHashByPrefix(f, fileHash);
}
- return false;
+ return true;
}
}
@@ -113,6 +127,29 @@ public class LocalMetadataStore implements MetadataStore {
}
}
+ @VisibleForTesting
+ static <T> void clearHashByPrefix(Path prefix, Map<Path,T> hash) {
+ for (Iterator<Map.Entry<Path,T>> it = hash.entrySet().iterator();
+ it.hasNext(); )
+ {
+ Map.Entry<Path,T> entry = it.next();
+ Path f = entry.getKey();
+ if (isAncestorOf(prefix, f)) {
+ it.remove();
+ }
+ }
+ }
+
+ /**
+ * @return true iff 'ancestor' is ancestor dir in path 'f'.
+ * Since all paths here are absolute, this is a prefix match.
+ */
+ private static boolean isAncestorOf(Path ancestor, Path f) {
+ String aStr = ancestor.toString();
+ String fStr = f.toString();
+ return (fStr.indexOf(aStr) == 0);
+ }
+
/**
* Update dirHash to reflect deletion of file 'f'. Call with lock held.
*/
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9d32cea2/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/MetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/MetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/MetadataStore.java
index 7baed90..a24cf76 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/MetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/MetadataStore.java
@@ -26,6 +26,12 @@ import javax.annotation.Nullable;
import java.io.FileNotFoundException;
import java.io.IOException;
+/**
+ * Interface for a coherent metadata cache, or "store".
+ * All Path arguments should be absolute, without URI scheme, e.g. "/a/b/c"
+ * or "/". TODO: should implementations enforce this, or give undefined
+ * behavior?
+ */
@InterfaceAudience.LimitedPrivate("HDFS")
@InterfaceStability.Unstable
public interface MetadataStore {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9d32cea2/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestLocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestLocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestLocalMetadataStore.java
index 54e96ac..499718c 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestLocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestLocalMetadataStore.java
@@ -12,6 +12,10 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
/**
* Unit tests for LocalMetadataStore.
@@ -121,6 +125,23 @@ public class TestLocalMetadataStore extends Assert {
}
+ /** Additional internal test for fiddly part of code. */
+ @Test
+ public void testClearByHashPrefix() throws Exception {
+ Map<Path,String> testMap = new HashMap<>();
+ List<String> paths = Arrays.asList("/a1", "/a1/b1", "/a1/b1/c1",
+ "/a1/b1/c1/d1/e1", // sparse cache might not include d1 entry
+ "/a2", "/a2/b2", "/a2/b2/c2");
+ for (String pathStr : paths) {
+ testMap.put(new Path(pathStr), "blah");
+ }
+ LocalMetadataStore.clearHashByPrefix(new Path("/a1/b1"), testMap);
+ assertEquals("Three were deleted", paths.size()-3, testMap.size());
+ assertNull("b1 deleted", testMap.get("/a1/b1"));
+ assertNull("c1 deleted", testMap.get("/a1/b1/c1"));
+ assertNull("e1 deleted", testMap.get("/a1/b1/c1/d1/e1"));
+ }
+
private void createNewDirs(String... dirs)
throws IOException {
for (String pathStr : dirs) {
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org