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 fa...@apache.org on 2016/10/29 00:19:17 UTC
hadoop git commit: HADOOP-13631 S3Guard: implement move() for
LocalMetadataStore, add unit tests
Repository: hadoop
Updated Branches:
refs/heads/HADOOP-13345 d7af9c515 -> 5d280b903
HADOOP-13631 S3Guard: implement move() for LocalMetadataStore, add unit tests
Also, change DirListingMetadata#setAuthoritative() to take a boolean arg.
Setting this to false is a way to trigger clients to re-consult the backing
store if, for example, a user adds new files to a directory.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5d280b90
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5d280b90
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5d280b90
Branch: refs/heads/HADOOP-13345
Commit: 5d280b9032a6f3b8fe6cc2516315cd39479acfdd
Parents: d7af9c5
Author: Aaron Fabbri <fa...@apache.org>
Authored: Tue Sep 27 18:19:48 2016 -0700
Committer: Aaron Fabbri <fa...@apache.org>
Committed: Fri Oct 28 17:18:30 2016 -0700
----------------------------------------------------------------------
.../fs/s3a/s3guard/DirListingMetadata.java | 5 +-
.../fs/s3a/s3guard/LocalMetadataStore.java | 44 +++++++++++--
.../hadoop/fs/s3a/s3guard/MetadataStore.java | 26 ++++++--
.../fs/s3a/s3guard/MetadataStoreTestBase.java | 69 ++++++++++++++++++--
.../fs/s3a/s3guard/TestDirListingMetadata.java | 2 +-
5 files changed, 127 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5d280b90/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java
index 1838f42..5bfb6c0 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java
@@ -97,9 +97,10 @@ public class DirListingMetadata {
/**
* Marks this directory listing as full and authoritative.
+ * @param authoritative see {@link #isAuthoritative()}.
*/
- public void setAuthoritative() {
- isAuthoritative = true;
+ public void setAuthoritative(boolean authoritative) {
+ this.isAuthoritative = authoritative;
}
/**
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5d280b90/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java
index d47d85e..9f69189 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java
@@ -28,6 +28,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
@@ -94,7 +95,7 @@ public class LocalMetadataStore implements MetadataStore {
// Remove target file/dir
fileHash.remove(path);
- // Update parent dir listing, if any
+ // Update this and parent dir listing, if any
dirHashDeleteFile(path);
if (recursive) {
@@ -116,9 +117,39 @@ public class LocalMetadataStore implements MetadataStore {
}
@Override
- public void move(Path src, Path dst) throws IOException {
- // TODO implement me
- throw new IOException("LocalMetadataStore#move() not implemented yet");
+ public void move(Collection<Path> pathsToDelete,
+ Collection<PathMetadata> pathsToCreate) throws IOException {
+
+ Preconditions.checkNotNull(pathsToDelete, "pathsToDelete is null");
+ Preconditions.checkNotNull(pathsToCreate, "pathsToCreate is null");
+ Preconditions.checkArgument(pathsToDelete.size() == pathsToCreate.size(),
+ "Must supply same number of paths to delete/create.");
+
+ // I feel dirty for using reentrant lock. :-|
+ synchronized (this) {
+
+ // 1. Delete pathsToDelete
+ for (Path p : pathsToDelete) {
+ delete(p);
+ }
+
+ // 2. Create new destination path metadata
+ for (PathMetadata meta : pathsToCreate) {
+ put(meta);
+ }
+
+ // 3. We now know full contents of all dirs in destination subtree
+ for (PathMetadata meta : pathsToCreate) {
+ FileStatus status = meta.getFileStatus();
+ if (status == null || status.isDirectory()) {
+ continue;
+ }
+ DirListingMetadata dir = listChildren(status.getPath());
+ if (dir != null) { // could be evicted already
+ dir.setAuthoritative(true);
+ }
+ }
+ }
}
@Override
@@ -205,6 +236,11 @@ public class LocalMetadataStore implements MetadataStore {
* Update dirHash to reflect deletion of file 'f'. Call with lock held.
*/
private void dirHashDeleteFile(Path path) {
+
+ /* If this path is a dir, remove its listing */
+ dirHash.remove(path);
+
+ /* Remove this path from parent's dir listing */
Path parent = path.getParent();
if (parent != null) {
DirListingMetadata dir = dirHash.get(parent);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5d280b90/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java
index a6d29da..68009e3 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.fs.s3a.s3guard;
import java.io.Closeable;
import java.io.IOException;
+import java.util.Collection;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
@@ -84,14 +85,29 @@ public interface MetadataStore extends Closeable {
DirListingMetadata listChildren(Path path) throws IOException;
/**
- * Moves metadata from {@code src} to {@code dst}, including all descendants
- * recursively.
+ * Record the effects of a {@link FileSystem#rename(Path, Path)} in the
+ * MetadataStore. Clients provide explicit enumeration of the affected
+ * paths (recursively), before and after the rename.
*
- * @param src the source path
- * @param dst the new path of the file/dir after the move
+ * On the need to provide an enumeration of directory trees instead of just
+ * source and destination paths:
+ * Since a MetadataStore does not have to track all metadata for the
+ * underlying storage system, and a new MetadataStore may be created on an
+ * existing underlying filesystem, this move() may be the first time the
+ * MetadataStore sees the affected paths. Therefore, simply providing src
+ * and destination paths may not be enough to record the deletions (under
+ * src path) and creations (at destination) that are happening during the
+ * rename().
+ *
+ * @param pathsToDelete Collection of all paths that were removed from the
+ * source directory tree of the move.
+ * @param pathsToCreate Collection of all PathMetadata for the new paths
+ * that were created at the destination of the rename
+ * ().
* @throws IOException if there is an error
*/
- void move(Path src, Path dst) throws IOException;
+ void move(Collection<Path> pathsToDelete, Collection<PathMetadata>
+ pathsToCreate) throws IOException;
/**
* Saves metadata for exactly one path. For a deeply nested path, this method
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5d280b90/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java
index 701f355..462f8f9 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java
@@ -24,7 +24,6 @@ import org.apache.hadoop.fs.permission.FsPermission;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -32,6 +31,7 @@ import org.slf4j.LoggerFactory;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
@@ -285,10 +285,54 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.listChildren(new Path("/a1/b1x")));
}
- @Ignore
+ @Test
public void testMove() throws Exception {
- // TODO
- throw new RuntimeException("TODO: implement move and tests.");
+ // Create test dir structure
+ createNewDirs("/a1", "/a2", "/a3");
+ createNewDirs("/a1/b1", "/a1/b2");
+ putListStatusFiles("/a1/b1", false, "/a1/b1/file1", "/a1/b1/file2");
+
+ // Assert root listing as expected
+ DirListingMetadata dirMeta = ms.listChildren(new Path("/"));
+ assertNotNull("Listing root", dirMeta);
+ Collection<PathMetadata> entries = dirMeta.getListing();
+ assertListingsEqual(entries, "/a1", "/a2", "/a3");
+
+ // Assert src listing as expected
+ dirMeta = ms.listChildren(new Path("/a1/b1"));
+ assertNotNull("Listing /a1/b1", dirMeta);
+ entries = dirMeta.getListing();
+ assertListingsEqual(entries, "/a1/b1/file1", "/a1/b1/file2");
+
+ // Do the move(): rename(/a1/b1, /b1)
+ Collection<Path> srcPaths = Arrays.asList(new Path("/a1/b1"),
+ new Path("/a1/b1/file1"), new Path("/a1/b1/file2"));
+
+ ArrayList<PathMetadata> destMetas = new ArrayList<>();
+ destMetas.add(new PathMetadata(makeDirStatus("/b1")));
+ destMetas.add(new PathMetadata(makeFileStatus("/b1/file1", 100)));
+ destMetas.add(new PathMetadata(makeFileStatus("/b1/file2", 100)));
+ ms.move(srcPaths, destMetas);
+
+ // Assert src is no longer there
+ dirMeta = ms.listChildren(new Path("/a1"));
+ assertNotNull("Listing /a1", dirMeta);
+ entries = dirMeta.getListing();
+ assertListingsEqual(entries, "/a1/b2");
+
+ PathMetadata meta = ms.get(new Path("/a1/b1/file1"));
+ // TODO allow return of PathMetadata with isDeleted == true
+ assertNull("Src path deleted", meta);
+
+ // Assert dest looks right
+ meta = ms.get(new Path("/b1/file1"));
+ assertNotNull("dest file not null", meta);
+ verifyBasicFileStatus(meta);
+
+ dirMeta = ms.listChildren(new Path("/b1"));
+ assertNotNull("dest listing not null", dirMeta);
+ entries = dirMeta.getListing();
+ assertListingsEqual(entries, "/b1/file1", "/b1/file2");
}
@@ -296,8 +340,18 @@ public abstract class MetadataStoreTestBase extends Assert {
* Helper functions.
*/
+ /** Builds array of Path objects based on parent dir and filenames. */
+ private Path[] buildPaths(String parent, String... paths) {
+
+ Path[] output = new Path[paths.length];
+ for (int i = 0; i < paths.length; i++) {
+ output[i] = new Path(parent, paths[i]);
+ }
+ return output;
+ }
+
/** Modifies paths input array and returns it. */
- private String[] buildPaths(String parent, String ...paths) {
+ private String[] buildPathStrings(String parent, String... paths) {
for (int i = 0; i < paths.length; i++) {
Path p = new Path(parent, paths[i]);
paths[i] = p.toString();
@@ -306,13 +360,14 @@ public abstract class MetadataStoreTestBase extends Assert {
}
private void commonTestPutListStatus(final String parent) throws IOException {
- putListStatusFiles(parent, true, buildPaths(parent, "file1", "file2",
+ putListStatusFiles(parent, true, buildPathStrings(parent, "file1", "file2",
"file3"));
DirListingMetadata dirMeta = ms.listChildren(new Path(parent));
assertNotNull("list after putListStatus", dirMeta);
Collection<PathMetadata> entries = dirMeta.getListing();
assertNotNull("listStatus has entries", entries);
- assertListingsEqual(entries, buildPaths(parent, "file1", "file2", "file3"));
+ assertListingsEqual(entries, buildPathStrings(parent, "file1", "file2",
+ "file3"));
}
private void setupListStatus() throws IOException {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5d280b90/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDirListingMetadata.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDirListingMetadata.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDirListingMetadata.java
index 99db001..05d3876 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDirListingMetadata.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDirListingMetadata.java
@@ -122,7 +122,7 @@ public class TestDirListingMetadata {
assertNotNull(meta.getListing());
assertTrue(meta.getListing().isEmpty());
assertFalse(meta.isAuthoritative());
- meta.setAuthoritative();
+ meta.setAuthoritative(true);
assertTrue(meta.isAuthoritative());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org