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 ma...@apache.org on 2017/05/25 12:13:13 UTC
[1/6] hadoop git commit: Revert "Reproduced erroneous authoritative
listing"
Repository: hadoop
Updated Branches:
refs/heads/HADOOP-13345 80613da01 -> c17759607
Revert "Reproduced erroneous authoritative listing"
This reverts commit 8e6e39f42045247c21f9546103f6f243de4e75a8.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/447d7b8f
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/447d7b8f
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/447d7b8f
Branch: refs/heads/HADOOP-13345
Commit: 447d7b8fa6fa309ff42103721a4e2ef6f303e76a
Parents: 83b8234
Author: Sean Mackrory <ma...@apache.org>
Authored: Wed May 24 14:49:26 2017 -0600
Committer: Sean Mackrory <ma...@apache.org>
Committed: Wed May 24 17:19:39 2017 -0600
----------------------------------------------------------------------
.../org/apache/hadoop/fs/s3a/S3AFileSystem.java | 2 +-
.../fs/contract/s3a/ITestS3AContractRename.java | 47 --------------------
.../fs/s3a/s3guard/TestLocalMetadataStore.java | 2 +-
3 files changed, 2 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/447d7b8f/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
index ffb2147..7de94cc 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
@@ -960,7 +960,7 @@ public class S3AFileSystem extends FileSystem {
}
@VisibleForTesting
- public MetadataStore getMetadataStore() {
+ MetadataStore getMetadataStore() {
return metadataStore;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/447d7b8f/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
index 65da13a..4339649 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
@@ -23,14 +23,6 @@ import org.apache.hadoop.fs.contract.AbstractContractRenameTest;
import org.apache.hadoop.fs.contract.AbstractFSContract;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.s3a.S3AFileSystem;
-import org.apache.hadoop.fs.s3a.Tristate;
-import org.apache.hadoop.fs.s3a.s3guard.DirListingMetadata;
-import org.apache.hadoop.fs.s3a.s3guard.MetadataStore;
-import org.apache.hadoop.fs.s3a.s3guard.S3Guard;
-import org.junit.Test;
-
-import java.io.IOException;
import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
@@ -80,43 +72,4 @@ public class ITestS3AContractRename extends AbstractContractRenameTest {
boolean rename = fs.rename(srcDir, destDir);
assertFalse("s3a doesn't support rename to non-empty directory", rename);
}
-
- @Override
- @Test
- public void testRenamePopulatesFileAncestors() throws IOException {
- final FileSystem fs = getFileSystem();
- final Path src = path("testRenamePopulatesFileAncestors/source");
- fs.mkdirs(src);
- final String nestedFile = "/dir1/dir2/dir3/file4";
- byte[] srcDataset = dataset(256, 'a', 'z');
- writeDataset(fs, path(src + nestedFile), srcDataset, srcDataset.length,
- 1024, false);
-
- Path dst = path("testRenamePopulatesFileAncestorsNew");
-
- DirListingMetadata l = ((S3AFileSystem)fs).getMetadataStore()
- .listChildren(src);
- fs.rename(src, dst);
- }
-
- @Test
- public void testMkdirPopulatesFileAncestors() throws IOException {
- final FileSystem fs = getFileSystem();
- final MetadataStore ms = ((S3AFileSystem) fs).getMetadataStore();
- final Path parent = path("testMkdirPopulatesFileAncestors/source");
- try {
- //fs.mkdirs(parent);
- final Path nestedFile = new Path(parent, "/dir1/dir2/dir3/file4");
- byte[] srcDataset = dataset(256, 'a', 'z');
- writeDataset(fs, nestedFile, srcDataset, srcDataset.length,
- 1024, false);
-
- DirListingMetadata list = ms.listChildren(parent);
- System.out.println("list isEmpty: " + list.isEmpty());
- System.out.println("list isAuthoritative: " + list.isAuthoritative());
- assertTrue(list.isEmpty() == Tristate.FALSE || !list.isAuthoritative());
- } finally {
- fs.delete(parent, true);
- }
- }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/447d7b8f/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
index 81183e70..89d0498 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
@@ -24,7 +24,6 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.S3ATestUtils;
-import org.apache.hadoop.fs.s3a.Tristate;
import org.junit.Test;
import java.io.IOException;
@@ -137,4 +136,5 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
S3ATestUtils.verifyDirStatus(status, REPLICATION, getModTime(),
getAccessTime(), OWNER, GROUP, PERMISSION);
}
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[4/6] hadoop git commit: HADOOP-13760. S3Guard: add delete tracking.
Posted by ma...@apache.org.
HADOOP-13760. S3Guard: add delete tracking.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/2f3305db
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/2f3305db
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/2f3305db
Branch: refs/heads/HADOOP-13345
Commit: 2f3305db768e4159b9216de88adbd73b686d9a02
Parents: 80613da
Author: Sean Mackrory <ma...@apache.org>
Authored: Fri Apr 7 17:21:58 2017 -0600
Committer: Sean Mackrory <ma...@apache.org>
Committed: Wed May 24 17:19:39 2017 -0600
----------------------------------------------------------------------
.../fs/s3a/InconsistentAmazonS3Client.java | 122 +++++++-
.../java/org/apache/hadoop/fs/s3a/Listing.java | 91 ++++++
.../org/apache/hadoop/fs/s3a/S3AFileSystem.java | 310 +++++++++++++------
.../fs/s3a/s3guard/DescendantsIterator.java | 14 +-
.../fs/s3a/s3guard/DirListingMetadata.java | 32 ++
.../fs/s3a/s3guard/DynamoDBMetadataStore.java | 86 +++--
.../fs/s3a/s3guard/LocalMetadataStore.java | 61 ++--
.../hadoop/fs/s3a/s3guard/MetadataStore.java | 19 +-
.../s3guard/MetadataStoreListFilesIterator.java | 168 ++++++++++
.../fs/s3a/s3guard/NullMetadataStore.java | 5 +
.../hadoop/fs/s3a/s3guard/PathMetadata.java | 27 +-
.../PathMetadataDynamoDBTranslation.java | 7 +-
.../apache/hadoop/fs/s3a/s3guard/S3Guard.java | 26 +-
.../hadoop/fs/s3a/s3guard/S3GuardTool.java | 5 +-
.../hadoop/fs/s3a/ITestS3GuardEmptyDirs.java | 62 ++--
.../fs/s3a/ITestS3GuardListConsistency.java | 213 ++++++++++++-
.../org/apache/hadoop/fs/s3a/TestListing.java | 94 ++++++
.../fs/s3a/s3guard/MetadataStoreTestBase.java | 150 ++++++---
.../s3a/s3guard/TestDynamoDBMetadataStore.java | 4 +-
.../fs/s3a/s3guard/TestLocalMetadataStore.java | 38 ++-
.../AbstractITestS3AMetadataStoreScale.java | 2 +-
21 files changed, 1260 insertions(+), 276 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
index ebca268..aadcc37 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
@@ -23,6 +23,7 @@ import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.services.s3.AmazonS3Client;
+import com.amazonaws.services.s3.model.DeleteObjectRequest;
import com.amazonaws.services.s3.model.ListObjectsRequest;
import com.amazonaws.services.s3.model.ObjectListing;
import com.amazonaws.services.s3.model.PutObjectRequest;
@@ -33,6 +34,7 @@ import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -57,14 +59,50 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
private static final Logger LOG =
LoggerFactory.getLogger(InconsistentAmazonS3Client.class);
+ /**
+ * Composite of data we need to track about recently deleted objects:
+ * when it was deleted (same was with recently put objects) and the object
+ * summary (since we should keep returning it for sometime after its
+ * deletion).
+ */
+ private static class Delete {
+ private Long time;
+ private S3ObjectSummary summary;
+
+ Delete(Long time, S3ObjectSummary summary) {
+ this.time = time;
+ this.summary = summary;
+ }
+
+ public Long time() {
+ return time;
+ }
+
+ public S3ObjectSummary summary() {
+ return summary;
+ }
+ }
+
+ /** Map of key to delay -> time it was deleted + object summary (object
+ * summary is null for prefixes. */
+ private Map<String, Delete> delayedDeletes = new HashMap<>();
+
/** Map of key to delay -> time it was created. */
- private Map<String, Long> delayedKeys = new HashMap<>();
+ private Map<String, Long> delayedPutKeys = new HashMap<>();
public InconsistentAmazonS3Client(AWSCredentialsProvider credentials,
ClientConfiguration clientConfiguration) {
super(credentials, clientConfiguration);
}
+ @Override
+ public void deleteObject(DeleteObjectRequest deleteObjectRequest)
+ throws AmazonClientException, AmazonServiceException {
+ LOG.debug("key {}", deleteObjectRequest.getKey());
+ registerDeleteObject(deleteObjectRequest);
+ super.deleteObject(deleteObjectRequest);
+ }
+
/* We should only need to override this version of putObject() */
@Override
public PutObjectResult putObject(PutObjectRequest putObjectRequest)
@@ -80,10 +118,46 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
throws AmazonClientException, AmazonServiceException {
LOG.debug("prefix {}", listObjectsRequest.getPrefix());
ObjectListing listing = super.listObjects(listObjectsRequest);
- return filterListObjects(listObjectsRequest,
- listing);
+ listing = filterListObjects(listObjectsRequest, listing);
+ listing = restoreListObjects(listObjectsRequest, listing);
+ return listing;
}
+ private boolean addIfNotPresent(List<S3ObjectSummary> list,
+ S3ObjectSummary item) {
+ // Behavior of S3ObjectSummary
+ String key = item.getKey();
+ for (S3ObjectSummary member : list) {
+ if (member.getKey().equals(key)) {
+ return false;
+ }
+ }
+ return list.add(item);
+ }
+
+ private ObjectListing restoreListObjects(ListObjectsRequest request,
+ ObjectListing rawListing) {
+ List<S3ObjectSummary> outputList = rawListing.getObjectSummaries();
+ List<String> outputPrefixes = rawListing.getCommonPrefixes();
+ for (String key : new HashSet<>(delayedDeletes.keySet())) {
+ Delete delete = delayedDeletes.get(key);
+ if (isKeyDelayed(delete.time(), key)) {
+ if (key.startsWith(request.getPrefix())) {
+ if (delete.summary == null) {
+ if (!outputPrefixes.contains(key)) {
+ outputPrefixes.add(key);
+ }
+ } else {
+ addIfNotPresent(outputList, delete.summary());
+ }
+ }
+ } else {
+ delayedDeletes.remove(key);
+ }
+ }
+
+ return new CustomObjectListing(rawListing, outputList, outputPrefixes);
+ }
private ObjectListing filterListObjects(ListObjectsRequest request,
ObjectListing rawListing) {
@@ -91,7 +165,8 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
// Filter object listing
List<S3ObjectSummary> outputList = new ArrayList<>();
for (S3ObjectSummary s : rawListing.getObjectSummaries()) {
- if (!isVisibilityDelayed(s.getKey())) {
+ String key = s.getKey();
+ if (!isKeyDelayed(delayedPutKeys.get(key), key)) {
outputList.add(s);
}
}
@@ -99,7 +174,7 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
// Filter prefixes (directories)
List<String> outputPrefixes = new ArrayList<>();
for (String key : rawListing.getCommonPrefixes()) {
- if (!isVisibilityDelayed(key)) {
+ if (!isKeyDelayed(delayedPutKeys.get(key), key)) {
outputPrefixes.add(key);
}
}
@@ -107,28 +182,43 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
return new CustomObjectListing(rawListing, outputList, outputPrefixes);
}
- private boolean isVisibilityDelayed(String key) {
- Long createTime = delayedKeys.get(key);
- if (createTime == null) {
+ private boolean isKeyDelayed(Long enqueueTime, String key) {
+ if (enqueueTime == null) {
LOG.debug("no delay for key {}", key);
return false;
}
long currentTime = System.currentTimeMillis();
- long deadline = createTime + DELAY_KEY_MILLIS;
+ long deadline = enqueueTime + DELAY_KEY_MILLIS;
if (currentTime >= deadline) {
- delayedKeys.remove(key);
- LOG.debug("{} no longer delayed", key);
+ delayedDeletes.remove(key);
+ LOG.debug("no longer delaying {}", key);
return false;
} else {
- LOG.info("{} delaying visibility", key);
+ LOG.info("delaying {}", key);
return true;
}
}
+ private void registerDeleteObject(DeleteObjectRequest req) {
+ String key = req.getKey();
+ if (shouldDelay(key)) {
+ // Record summary so we can add it back for some time post-deletion
+ S3ObjectSummary summary = null;
+ ObjectListing list = listObjects(req.getBucketName(), key);
+ for (S3ObjectSummary result : list.getObjectSummaries()) {
+ if (result.getKey().equals(key)) {
+ summary = result;
+ break;
+ }
+ }
+ delayedDeletes.put(key, new Delete(System.currentTimeMillis(), summary));
+ }
+ }
+
private void registerPutObject(PutObjectRequest req) {
String key = req.getKey();
if (shouldDelay(key)) {
- enqueueDelayKey(key);
+ enqueueDelayedPut(key);
}
}
@@ -148,9 +238,9 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
* listObject replies for a while, to simulate eventual list consistency.
* @param key key to delay visibility of
*/
- private void enqueueDelayKey(String key) {
- LOG.debug("key {}", key);
- delayedKeys.put(key, System.currentTimeMillis());
+ private void enqueueDelayedPut(String key) {
+ LOG.debug("delaying put of {}", key);
+ delayedPutKeys.put(key, System.currentTimeMillis());
}
/** Since ObjectListing is immutable, we just override it with wrapper. */
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
index e91f2ec..5da99b8 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
@@ -22,6 +22,7 @@ import com.amazonaws.AmazonClientException;
import com.amazonaws.services.s3.model.ListObjectsRequest;
import com.amazonaws.services.s3.model.ObjectListing;
import com.amazonaws.services.s3.model.S3ObjectSummary;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.LocatedFileStatus;
import org.apache.hadoop.fs.Path;
@@ -33,6 +34,7 @@ import org.slf4j.Logger;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -125,12 +127,27 @@ public class Listing {
* @param statusIterator an iterator over the remote status entries
* @return a new remote iterator
*/
+ @VisibleForTesting
LocatedFileStatusIterator createLocatedFileStatusIterator(
RemoteIterator<FileStatus> statusIterator) {
return new LocatedFileStatusIterator(statusIterator);
}
/**
+ * Create an located status iterator that wraps another to filter out a set
+ * of recently deleted items.
+ * @param iterator an iterator over the remote located status entries.
+ * @param tombstones set of paths that are recently deleted and should be
+ * filtered.
+ * @return a new remote iterator.
+ */
+ @VisibleForTesting
+ TombstoneReconcilingIterator createTombstoneReconcilingIterator(
+ RemoteIterator<LocatedFileStatus> iterator, Set<Path> tombstones) {
+ return new TombstoneReconcilingIterator(iterator, tombstones);
+ }
+
+ /**
* Interface to implement by the logic deciding whether to accept a summary
* entry or path as a valid file or directory.
*/
@@ -668,6 +685,80 @@ public class Listing {
}
/**
+ * Wraps another iterator and filters out files that appear in the provided
+ * set of tombstones. Will read ahead in the iterator when necessary to
+ * ensure that emptiness is detected early enough if only deleted objects
+ * remain in the source iterator.
+ */
+ static class TombstoneReconcilingIterator implements
+ RemoteIterator<LocatedFileStatus> {
+ private LocatedFileStatus next = null;
+ private final RemoteIterator<LocatedFileStatus> iterator;
+ private final Set<Path> tombstones;
+
+ /**
+ * @param iterator Source iterator to filter
+ * @param tombstones set of tombstone markers to filter out of results
+ */
+ public TombstoneReconcilingIterator(RemoteIterator<LocatedFileStatus>
+ iterator, Set<Path> tombstones) {
+ this.iterator = iterator;
+ if (tombstones != null) {
+ this.tombstones = tombstones;
+ } else {
+ this.tombstones = Collections.EMPTY_SET;
+ }
+ }
+
+ private boolean fetch() throws IOException {
+ while (next == null && iterator.hasNext()) {
+ LocatedFileStatus candidate = iterator.next();
+ if (!tombstones.contains(candidate.getPath())) {
+ next = candidate;
+ return true;
+ }
+ }
+ return false;
+ }
+
+ public boolean hasNext() throws IOException {
+ if (next != null) {
+ return true;
+ }
+ return fetch();
+ }
+
+ public LocatedFileStatus next() throws IOException {
+ if (hasNext()) {
+ LocatedFileStatus result = next;
+ next = null;
+ fetch();
+ return result;
+ }
+ throw new NoSuchElementException();
+ }
+ }
+
+ /**
+ * Accept all entries except those which map to S3N pseudo directory markers.
+ */
+ static class AcceptAllButS3nDirs implements FileStatusAcceptor {
+
+ public boolean accept(Path keyPath, S3ObjectSummary summary) {
+ return !summary.getKey().endsWith(S3N_FOLDER_SUFFIX);
+ }
+
+ public boolean accept(Path keyPath, String prefix) {
+ return !keyPath.toString().endsWith(S3N_FOLDER_SUFFIX);
+ }
+
+ public boolean accept(FileStatus status) {
+ return !status.getPath().toString().endsWith(S3N_FOLDER_SUFFIX);
+ }
+
+ }
+
+ /**
* Accept all entries except the base path and those which map to S3N
* pseudo directory markers.
*/
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
index 78b3970..7de94cc 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
@@ -26,11 +26,13 @@ import java.io.InterruptedIOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Date;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.Objects;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.LinkedBlockingQueue;
@@ -93,8 +95,8 @@ import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.StorageStatistics;
import org.apache.hadoop.fs.permission.FsPermission;
-import org.apache.hadoop.fs.s3a.s3guard.DescendantsIterator;
import org.apache.hadoop.fs.s3a.s3guard.DirListingMetadata;
+import org.apache.hadoop.fs.s3a.s3guard.MetadataStoreListFilesIterator;
import org.apache.hadoop.fs.s3a.s3guard.MetadataStore;
import org.apache.hadoop.fs.s3a.s3guard.PathMetadata;
import org.apache.hadoop.fs.s3a.s3guard.S3Guard;
@@ -878,51 +880,44 @@ public class S3AFileSystem extends FileSystem {
keysToDelete.add(new DeleteObjectsRequest.KeyVersion(dstKey));
}
- ListObjectsRequest request = new ListObjectsRequest();
- request.setBucketName(bucket);
- request.setPrefix(srcKey);
- request.setMaxKeys(maxKeys);
-
- ObjectListing objects = listObjects(request);
-
- while (true) {
- for (S3ObjectSummary summary : objects.getObjectSummaries()) {
- long length = summary.getSize();
- keysToDelete
- .add(new DeleteObjectsRequest.KeyVersion(summary.getKey()));
- String newDstKey =
- dstKey + summary.getKey().substring(srcKey.length());
- copyFile(summary.getKey(), newDstKey, length);
-
- if (hasMetadataStore()) {
- Path childSrc = keyToQualifiedPath(summary.getKey());
- Path childDst = keyToQualifiedPath(newDstKey);
- if (objectRepresentsDirectory(summary.getKey(), length)) {
- S3Guard.addMoveDir(metadataStore, srcPaths, dstMetas, childSrc,
- childDst, username);
- } else {
- S3Guard.addMoveFile(metadataStore, srcPaths, dstMetas, childSrc,
- childDst, length, getDefaultBlockSize(childDst), username);
- }
- // Ancestor directories may not be listed, so we explicitly add them
- S3Guard.addMoveAncestors(metadataStore, srcPaths, dstMetas,
- keyToQualifiedPath(srcKey), childSrc, childDst, username);
- }
-
- if (keysToDelete.size() == MAX_ENTRIES_TO_DELETE) {
- removeKeys(keysToDelete, true, false);
+ Path parentPath = keyToPath(srcKey);
+ RemoteIterator<LocatedFileStatus> iterator = listFilesAndEmptyDirectories(
+ parentPath, true);
+ while (iterator.hasNext()) {
+ LocatedFileStatus status = iterator.next();
+ long length = status.getLen();
+ String key = pathToKey(status.getPath());
+ if (status.isDirectory() && !key.endsWith("/")) {
+ key += "/";
+ }
+ keysToDelete
+ .add(new DeleteObjectsRequest.KeyVersion(key));
+ String newDstKey =
+ dstKey + key.substring(srcKey.length());
+ copyFile(key, newDstKey, length);
+
+ if (hasMetadataStore()) {
+ Path childSrc = keyToQualifiedPath(key);
+ Path childDst = keyToQualifiedPath(newDstKey);
+ if (objectRepresentsDirectory(key, length)) {
+ S3Guard.addMoveDir(metadataStore, srcPaths, dstMetas, childSrc,
+ childDst, username);
+ } else {
+ S3Guard.addMoveFile(metadataStore, srcPaths, dstMetas, childSrc,
+ childDst, length, getDefaultBlockSize(childDst), username);
}
+ // Ancestor directories may not be listed, so we explicitly add them
+ S3Guard.addMoveAncestors(metadataStore, srcPaths, dstMetas,
+ keyToQualifiedPath(srcKey), childSrc, childDst, username);
}
- if (objects.isTruncated()) {
- objects = continueListObjects(objects);
- } else {
- if (!keysToDelete.isEmpty()) {
- removeKeys(keysToDelete, false, false);
- }
- break;
+ if (keysToDelete.size() == MAX_ENTRIES_TO_DELETE) {
+ removeKeys(keysToDelete, true, false);
}
}
+ if (!keysToDelete.isEmpty()) {
+ removeKeys(keysToDelete, false, false);
+ }
// We moved all the children, now move the top-level dir
// Empty directory should have been added as the object summary
@@ -1632,6 +1627,42 @@ public class S3AFileSystem extends FileSystem {
throw translateException("innerMkdirs", path, e);
}
}
+
+ private enum DirectoryStatus {
+ DOES_NOT_EXIST, EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY,
+ EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE, EXISTS_AND_IS_FILE
+ }
+
+ private DirectoryStatus checkPathForDirectory(Path path) throws
+ IOException {
+ try {
+ if (path.isRoot()) {
+ return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE;
+ }
+ String key = pathToKey(path);
+
+ // Check MetadataStore, if any.
+ FileStatus status = null;
+ PathMetadata pm = metadataStore.get(path, false);
+ if (pm != null) {
+ if (pm.isDeleted()) {
+ return DirectoryStatus.DOES_NOT_EXIST;
+ }
+ status = pm.getFileStatus();
+ if (status != null && status.isDirectory()) {
+ return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE;
+ }
+ }
+ status = s3GetFileStatus(path, key, null);
+ if (status.isDirectory()) {
+ return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY;
+ }
+ } catch (FileNotFoundException e) {
+ return DirectoryStatus.DOES_NOT_EXIST;
+ }
+ return DirectoryStatus.EXISTS_AND_IS_FILE;
+ }
+
/**
*
* Make the given path and all non-existent parents into
@@ -1639,64 +1670,70 @@ public class S3AFileSystem extends FileSystem {
* See {@link #mkdirs(Path, FsPermission)}
* @param p path to create
* @param permission to apply to f
- * @return true if a directory was created
+ * @return true if a directory was created or already existed
* @throws FileAlreadyExistsException there is a file at the path specified
* @throws IOException other IO problems
* @throws AmazonClientException on failures inside the AWS SDK
*/
private boolean innerMkdirs(Path p, FsPermission permission)
throws IOException, FileAlreadyExistsException, AmazonClientException {
+ boolean createOnS3 = false;
Path f = qualify(p);
LOG.debug("Making directory: {}", f);
incrementStatistic(INVOCATION_MKDIRS);
- FileStatus fileStatus;
List<Path> metadataStoreDirs = null;
if (hasMetadataStore()) {
metadataStoreDirs = new ArrayList<>();
}
- try {
- fileStatus = getFileStatus(f);
-
- if (fileStatus.isDirectory()) {
- return true;
- } else {
- throw new FileAlreadyExistsException("Path is a file: " + f);
+ DirectoryStatus status = checkPathForDirectory(f);
+ if (status == DirectoryStatus.DOES_NOT_EXIST) {
+ createOnS3 = true;
+ if (metadataStoreDirs != null) {
+ metadataStoreDirs.add(f);
}
- } catch (FileNotFoundException e) {
- // Walk path to root, ensuring closest ancestor is a directory, not file
- Path fPart = f.getParent();
+ } else if (status == DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY) {
if (metadataStoreDirs != null) {
metadataStoreDirs.add(f);
}
- do {
- try {
- fileStatus = getFileStatus(fPart);
- if (fileStatus.isDirectory()) {
- break;
- }
- if (fileStatus.isFile()) {
- throw new FileAlreadyExistsException(String.format(
- "Can't make directory for path '%s' since it is a file.",
- fPart));
- }
- } catch (FileNotFoundException fnfe) {
- instrumentation.errorIgnored();
- // We create all missing directories in MetadataStore; it does not
- // infer directories exist by prefix like S3.
- if (metadataStoreDirs != null) {
- metadataStoreDirs.add(fPart);
- }
+ } else if (status == DirectoryStatus
+ .EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE) {
+ return true;
+ } else if (status == DirectoryStatus.EXISTS_AND_IS_FILE) {
+ throw new FileAlreadyExistsException("Path is a file: " + f);
+ }
+
+ // Walk path to root, ensuring closest ancestor is a directory, not file
+ Path fPart = f.getParent();
+ do {
+ status = checkPathForDirectory(fPart);
+ // The fake directory on S3 may not be visible immediately, but
+ // only the leaf node has a fake directory on S3 so we can treat both
+ // cases the same and just create a metadata store entry.
+ if (status == DirectoryStatus.DOES_NOT_EXIST || status ==
+ DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY) {
+ if (metadataStoreDirs != null) {
+ metadataStoreDirs.add(fPart);
}
- fPart = fPart.getParent();
- } while (fPart != null);
+ } else if (status == DirectoryStatus
+ .EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE) {
+ // do nothing - just break out of the loop, make whatever child
+ // directories are needed on the metadata store, and return the
+ // result.
+ break;
+ } else if (status == DirectoryStatus.EXISTS_AND_IS_FILE) {
+ throw new FileAlreadyExistsException("Path is a file: " + f);
+ }
+ fPart = fPart.getParent();
+ } while (fPart != null);
+ if (createOnS3) {
String key = pathToKey(f);
createFakeDirectory(key);
- S3Guard.makeDirsOrdered(metadataStore, metadataStoreDirs, username);
- deleteUnnecessaryFakeDirectories(f.getParent());
- return true;
}
+ S3Guard.makeDirsOrdered(metadataStore, metadataStoreDirs, username);
+ deleteUnnecessaryFakeDirectories(f.getParent());
+ return true;
}
/**
@@ -1729,8 +1766,12 @@ public class S3AFileSystem extends FileSystem {
// Check MetadataStore, if any.
PathMetadata pm = metadataStore.get(path, needEmptyDirectoryFlag);
+ Set<Path> tombstones = Collections.EMPTY_SET;
if (pm != null) {
- // HADOOP-13760: handle deleted files, i.e. PathMetadata#isDeleted() here
+ if (pm.isDeleted()) {
+ throw new FileNotFoundException("Path " + f + " is recorded as " +
+ "deleted by S3Guard");
+ }
FileStatus msStatus = pm.getFileStatus();
if (needEmptyDirectoryFlag && msStatus.isDirectory()) {
@@ -1738,15 +1779,29 @@ public class S3AFileSystem extends FileSystem {
// We have a definitive true / false from MetadataStore, we are done.
return S3AFileStatus.fromFileStatus(msStatus, pm.isEmptyDirectory());
} else {
+ DirListingMetadata children = metadataStore.listChildren(path);
+ if (children != null) {
+ tombstones = children.listTombstones();
+ }
LOG.debug("MetadataStore doesn't know if dir is empty, using S3.");
}
} else {
// Either this is not a directory, or we don't care if it is empty
return S3AFileStatus.fromFileStatus(msStatus, pm.isEmptyDirectory());
}
+
+ // If the metadata store has no children for it and it's not listed in
+ // S3 yet, we'll assume the empty directory is true;
+ S3AFileStatus s3FileStatus;
+ try {
+ s3FileStatus = s3GetFileStatus(path, key, tombstones);
+ } catch (FileNotFoundException e) {
+ return S3AFileStatus.fromFileStatus(msStatus, Tristate.TRUE);
+ }
+ return S3Guard.putAndReturn(metadataStore, s3FileStatus, instrumentation);
}
- return S3Guard.putAndReturn(metadataStore, s3GetFileStatus(path, key),
- instrumentation);
+ return S3Guard.putAndReturn(metadataStore,
+ s3GetFileStatus(path, key, tombstones), instrumentation);
}
/**
@@ -1757,8 +1812,8 @@ public class S3AFileSystem extends FileSystem {
* @return Status
* @throws IOException
*/
- private S3AFileStatus s3GetFileStatus(final Path path, String key)
- throws IOException {
+ private S3AFileStatus s3GetFileStatus(final Path path, String key,
+ Set<Path> tombstones) throws IOException {
if (!key.isEmpty()) {
try {
ObjectMetadata meta = getObjectMetadata(key);
@@ -1821,17 +1876,18 @@ public class S3AFileSystem extends FileSystem {
ObjectListing objects = listObjects(request);
- if (!objects.getCommonPrefixes().isEmpty()
- || !objects.getObjectSummaries().isEmpty()) {
+ Collection<String> prefixes = objects.getCommonPrefixes();
+ Collection<S3ObjectSummary> summaries = objects.getObjectSummaries();
+ if (!isEmptyOfKeys(prefixes, tombstones) ||
+ !isEmptyOfObjects(summaries, tombstones)) {
if (LOG.isDebugEnabled()) {
LOG.debug("Found path as directory (with /): {}/{}",
- objects.getCommonPrefixes().size() ,
- objects.getObjectSummaries().size());
+ prefixes.size(), summaries.size());
- for (S3ObjectSummary summary : objects.getObjectSummaries()) {
+ for (S3ObjectSummary summary : summaries) {
LOG.debug("Summary: {} {}", summary.getKey(), summary.getSize());
}
- for (String prefix : objects.getCommonPrefixes()) {
+ for (String prefix : prefixes) {
LOG.debug("Prefix: {}", prefix);
}
}
@@ -1853,6 +1909,48 @@ public class S3AFileSystem extends FileSystem {
throw new FileNotFoundException("No such file or directory: " + path);
}
+ /** Helper function to determine if a collection of paths is empty
+ * after accounting for tombstone markers (if provided).
+ * @param keys Collection of path (prefixes / directories or keys).
+ * @param tombstones Set of tombstone markers, or null if not applicable.
+ * @return false if summaries contains objects not accounted for by
+ * tombstones.
+ */
+ private boolean isEmptyOfKeys(Collection<String> keys, Set<Path>
+ tombstones) {
+ if (tombstones == null) {
+ return keys.isEmpty();
+ }
+ for (String key : keys) {
+ Path qualified = keyToQualifiedPath(key);
+ if (!tombstones.contains(qualified)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /** Helper function to determine if a collection of object summaries is empty
+ * after accounting for tombstone markers (if provided).
+ * @param summaries Collection of objects as returned by listObjects.
+ * @param tombstones Set of tombstone markers, or null if not applicable.
+ * @return false if summaries contains objects not accounted for by
+ * tombstones.
+ */
+ private boolean isEmptyOfObjects(Collection<S3ObjectSummary> summaries,
+ Set<Path> tombstones) {
+ if (tombstones == null) {
+ return summaries.isEmpty();
+ }
+ Collection<String> stringCollection = new ArrayList<>(summaries.size());
+ for(S3ObjectSummary summary : summaries) {
+ stringCollection.add(summary.getKey());
+ }
+ return isEmptyOfKeys(stringCollection, tombstones);
+ }
+
+
+
/**
* Raw version of {@link FileSystem#exists(Path)} which uses S3 only:
* S3Guard MetadataStore, if any, will be skipped.
@@ -1862,7 +1960,7 @@ public class S3AFileSystem extends FileSystem {
Path path = qualify(f);
String key = pathToKey(path);
try {
- return s3GetFileStatus(path, key) != null;
+ return s3GetFileStatus(path, key, null) != null;
} catch (FileNotFoundException e) {
return false;
}
@@ -2420,10 +2518,10 @@ public class S3AFileSystem extends FileSystem {
new Listing.AcceptFilesOnly(qualify(f)));
}
- public RemoteIterator<LocatedFileStatus> listFilesAndDirectories(Path f,
+ public RemoteIterator<LocatedFileStatus> listFilesAndEmptyDirectories(Path f,
boolean recursive) throws IOException {
return innerListFiles(f, recursive,
- new Listing.AcceptAllButSelfAndS3nDirs(qualify(f)));
+ new Listing.AcceptAllButS3nDirs());
}
private RemoteIterator<LocatedFileStatus> innerListFiles(Path f, boolean
@@ -2446,23 +2544,37 @@ public class S3AFileSystem extends FileSystem {
LOG.debug("Requesting all entries under {} with delimiter '{}'",
key, delimiter);
final RemoteIterator<FileStatus> cachedFilesIterator;
+ final Set<Path> tombstones;
if (recursive) {
final PathMetadata pm = metadataStore.get(path, true);
- cachedFilesIterator = new DescendantsIterator(metadataStore, pm);
+ // shouldn't need to check pm.isDeleted() because that will have
+ // been caught by getFileStatus above.
+ MetadataStoreListFilesIterator metadataStoreListFilesIterator =
+ new MetadataStoreListFilesIterator(metadataStore, pm,
+ allowAuthoritative);
+ tombstones = metadataStoreListFilesIterator.listTombstones();
+ cachedFilesIterator = metadataStoreListFilesIterator;
} else {
- final DirListingMetadata meta = metadataStore.listChildren(path);
+ DirListingMetadata meta = metadataStore.listChildren(path);
+ if (meta != null) {
+ tombstones = meta.listTombstones();
+ } else {
+ tombstones = null;
+ }
cachedFilesIterator = listing.createProvidedFileStatusIterator(
S3Guard.dirMetaToStatuses(meta), ACCEPT_ALL, acceptor);
if (allowAuthoritative && meta != null && meta.isAuthoritative()) {
return listing.createLocatedFileStatusIterator(cachedFilesIterator);
}
}
- return listing.createLocatedFileStatusIterator(
- listing.createFileStatusListingIterator(path,
- createListObjectsRequest(key, delimiter),
- ACCEPT_ALL,
- acceptor,
- cachedFilesIterator));
+ return listing.createTombstoneReconcilingIterator(
+ listing.createLocatedFileStatusIterator(
+ listing.createFileStatusListingIterator(path,
+ createListObjectsRequest(key, delimiter),
+ ACCEPT_ALL,
+ acceptor,
+ cachedFilesIterator)),
+ tombstones);
}
} catch (AmazonClientException e) {
// TODO s3guard:
@@ -2514,7 +2626,7 @@ public class S3AFileSystem extends FileSystem {
final String key = maybeAddTrailingSlash(pathToKey(path));
final Listing.FileStatusAcceptor acceptor =
new Listing.AcceptAllButSelfAndS3nDirs(path);
- final DirListingMetadata meta = metadataStore.listChildren(path);
+ DirListingMetadata meta = metadataStore.listChildren(path);
final RemoteIterator<FileStatus> cachedFileStatusIterator =
listing.createProvidedFileStatusIterator(
S3Guard.dirMetaToStatuses(meta), filter, acceptor);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
index d008972..262a6fa 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
@@ -19,6 +19,7 @@
package org.apache.hadoop.fs.s3a.s3guard;
import java.io.IOException;
+import java.util.Collection;
import java.util.LinkedList;
import java.util.NoSuchElementException;
import java.util.Queue;
@@ -103,8 +104,9 @@ public class DescendantsIterator implements RemoteIterator<FileStatus> {
if (meta != null) {
final Path path = meta.getFileStatus().getPath();
if (path.isRoot()) {
- final DirListingMetadata rootListing = ms.listChildren(path);
+ DirListingMetadata rootListing = ms.listChildren(path);
if (rootListing != null) {
+ rootListing = rootListing.withoutTombstones();
queue.addAll(rootListing.getListing());
}
} else {
@@ -123,11 +125,17 @@ public class DescendantsIterator implements RemoteIterator<FileStatus> {
if (!hasNext()) {
throw new NoSuchElementException("No more descendants.");
}
- final PathMetadata next;
+ PathMetadata next;
next = queue.poll();
if (next.getFileStatus().isDirectory()) {
final Path path = next.getFileStatus().getPath();
- queue.addAll(metadataStore.listChildren(path).getListing());
+ DirListingMetadata meta = metadataStore.listChildren(path);
+ if (meta != null) {
+ Collection<PathMetadata> more = meta.withoutTombstones().getListing();
+ if (!more.isEmpty()) {
+ queue.addAll(more);
+ }
+ }
}
return next.getFileStatus();
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/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 f13b447..320fa8d 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
@@ -25,9 +25,12 @@ import org.apache.hadoop.fs.Path;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import com.google.common.base.Preconditions;
@@ -104,6 +107,26 @@ public class DirListingMetadata {
return Collections.unmodifiableCollection(listMap.values());
}
+ public Set<Path> listTombstones() {
+ Set<Path> tombstones = new HashSet<>();
+ for (PathMetadata meta : listMap.values()) {
+ if (meta.isDeleted()) {
+ tombstones.add(meta.getFileStatus().getPath());
+ }
+ }
+ return tombstones;
+ }
+
+ public DirListingMetadata withoutTombstones() {
+ Collection<PathMetadata> filteredList = new ArrayList<>();
+ for (PathMetadata meta : listMap.values()) {
+ if (!meta.isDeleted()) {
+ filteredList.add(meta);
+ }
+ }
+ return new DirListingMetadata(path, filteredList, isAuthoritative);
+ }
+
/**
* @return number of entries tracked. This is not the same as the number
* of entries in the actual directory unless {@link #isAuthoritative()} is
@@ -166,6 +189,15 @@ public class DirListingMetadata {
}
/**
+ * Replace an entry with a tombstone.
+ * @param childPath path of entry to replace.
+ */
+ public void markDeleted(Path childPath) {
+ checkChildPath(childPath);
+ listMap.put(childPath, PathMetadata.tombstone(childPath));
+ }
+
+ /**
* Remove entry from this directory.
*
* @param childPath path of entry to remove.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
index 302541c..784b815 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
@@ -185,6 +185,9 @@ public class DynamoDBMetadataStore implements MetadataStore {
* DynamoDB. Value is {@value} msec. */
public static final long MIN_RETRY_SLEEP_MSEC = 100;
+ private static ValueMap deleteTrackingValueMap =
+ new ValueMap().withBoolean(":false", false);
+
private DynamoDB dynamoDB;
private String region;
private Table table;
@@ -286,6 +289,16 @@ public class DynamoDBMetadataStore implements MetadataStore {
@Override
public void delete(Path path) throws IOException {
+ innerDelete(path, true);
+ }
+
+ @Override
+ public void forgetMetadata(Path path) throws IOException {
+ innerDelete(path, false);
+ }
+
+ private void innerDelete(Path path, boolean tombstone)
+ throws IOException {
path = checkPath(path);
LOG.debug("Deleting from table {} in region {}: {}",
tableName, region, path);
@@ -297,7 +310,13 @@ public class DynamoDBMetadataStore implements MetadataStore {
}
try {
- table.deleteItem(pathToKey(path));
+ if (tombstone) {
+ Item item = PathMetadataDynamoDBTranslation.pathMetadataToItem(
+ PathMetadata.tombstone(path));
+ table.putItem(item);
+ } else {
+ table.deleteItem(pathToKey(path));
+ }
} catch (AmazonClientException e) {
throw translateException("delete", path, e);
}
@@ -310,17 +329,24 @@ public class DynamoDBMetadataStore implements MetadataStore {
tableName, region, path);
final PathMetadata meta = get(path);
- if (meta == null) {
+ if (meta == null || meta.isDeleted()) {
LOG.debug("Subtree path {} does not exist; this will be a no-op", path);
return;
}
for (DescendantsIterator desc = new DescendantsIterator(this, meta);
desc.hasNext();) {
- delete(desc.next().getPath());
+ innerDelete(desc.next().getPath(), true);
}
}
+ private Item getConsistentItem(PrimaryKey key) {
+ final GetItemSpec spec = new GetItemSpec()
+ .withPrimaryKey(key)
+ .withConsistentRead(true); // strictly consistent read
+ return table.getItem(spec);
+ }
+
@Override
public PathMetadata get(Path path) throws IOException {
return get(path, false);
@@ -338,10 +364,7 @@ public class DynamoDBMetadataStore implements MetadataStore {
// Root does not persist in the table
meta = new PathMetadata(makeDirStatus(username, path));
} else {
- final GetItemSpec spec = new GetItemSpec()
- .withPrimaryKey(pathToKey(path))
- .withConsistentRead(true); // strictly consistent read
- final Item item = table.getItem(spec);
+ final Item item = getConsistentItem(pathToKey(path));
meta = itemToPathMetadata(item, username);
LOG.debug("Get from table {} in region {} returning for {}: {}",
tableName, region, path, meta);
@@ -354,7 +377,8 @@ public class DynamoDBMetadataStore implements MetadataStore {
final QuerySpec spec = new QuerySpec()
.withHashKey(pathToParentKeyAttribute(path))
.withConsistentRead(true)
- .withMaxResultSize(1); // limit 1
+ .withFilterExpression(IS_DELETED + " = :false")
+ .withValueMap(deleteTrackingValueMap);
final ItemCollection<QueryOutcome> items = table.query(spec);
boolean hasChildren = items.iterator().hasNext();
// When this class has support for authoritative
@@ -398,7 +422,8 @@ public class DynamoDBMetadataStore implements MetadataStore {
final List<PathMetadata> metas = new ArrayList<>();
for (Item item : items) {
- metas.add(itemToPathMetadata(item, username));
+ PathMetadata meta = itemToPathMetadata(item, username);
+ metas.add(meta);
}
LOG.trace("Listing table {} in region {} for {} returning {}",
tableName, region, path, metas);
@@ -430,9 +455,9 @@ public class DynamoDBMetadataStore implements MetadataStore {
// Following code is to maintain this invariant by putting all ancestor
// directories of the paths to create.
// ancestor paths that are not explicitly added to paths to create
- Collection<PathMetadata> inferredPathsToCreate = null;
+ Collection<PathMetadata> newItems = new ArrayList<>();
if (pathsToCreate != null) {
- inferredPathsToCreate = new ArrayList<>(pathsToCreate);
+ newItems.addAll(pathsToCreate);
// help set for fast look up; we should avoid putting duplicate paths
final Collection<Path> fullPathsToCreate = new HashSet<>();
for (PathMetadata meta : pathsToCreate) {
@@ -448,16 +473,20 @@ public class DynamoDBMetadataStore implements MetadataStore {
LOG.debug("move: auto-create ancestor path {} for child path {}",
parent, meta.getFileStatus().getPath());
final FileStatus status = makeDirStatus(parent, username);
- inferredPathsToCreate.add(new PathMetadata(status, Tristate.FALSE));
+ newItems.add(new PathMetadata(status, Tristate.FALSE, false));
fullPathsToCreate.add(parent);
parent = parent.getParent();
}
}
}
+ if (pathsToDelete != null) {
+ for (Path meta : pathsToDelete) {
+ newItems.add(PathMetadata.tombstone(meta));
+ }
+ }
try {
- processBatchWriteRequest(pathToKey(pathsToDelete),
- pathMetadataToItem(inferredPathsToCreate));
+ processBatchWriteRequest(null, pathMetadataToItem(newItems));
} catch (AmazonClientException e) {
throw translateException("move", (String) null, e);
}
@@ -565,13 +594,10 @@ public class DynamoDBMetadataStore implements MetadataStore {
// first existent ancestor
Path path = meta.getFileStatus().getPath().getParent();
while (path != null && !path.isRoot()) {
- final GetItemSpec spec = new GetItemSpec()
- .withPrimaryKey(pathToKey(path))
- .withConsistentRead(true); // strictly consistent read
- final Item item = table.getItem(spec);
- if (item == null) {
+ final Item item = getConsistentItem(pathToKey(path));
+ if (!itemExists(item)) {
final FileStatus status = makeDirStatus(path, username);
- metasToPut.add(new PathMetadata(status, Tristate.FALSE));
+ metasToPut.add(new PathMetadata(status, Tristate.FALSE, false));
path = path.getParent();
} else {
break;
@@ -580,6 +606,17 @@ public class DynamoDBMetadataStore implements MetadataStore {
return metasToPut;
}
+ private boolean itemExists(Item item) {
+ if (item == null) {
+ return false;
+ }
+ if (item.hasAttribute(IS_DELETED) &&
+ item.getBoolean(IS_DELETED)) {
+ return false;
+ }
+ return true;
+ }
+
/** Create a directory FileStatus using current system time as mod time. */
static FileStatus makeDirStatus(Path f, String owner) {
return new FileStatus(0, true, 1, 0, System.currentTimeMillis(), 0,
@@ -591,9 +628,8 @@ public class DynamoDBMetadataStore implements MetadataStore {
LOG.debug("Saving to table {} in region {}: {}", tableName, region, meta);
// directory path
- PathMetadata p = new PathMetadata(
- makeDirStatus(meta.getPath(), username),
- meta.isEmpty());
+ PathMetadata p = new PathMetadata(makeDirStatus(meta.getPath(), username),
+ meta.isEmpty(), false);
// First add any missing ancestors...
final Collection<PathMetadata> metasToPut = fullPathsToPut(p);
@@ -670,13 +706,13 @@ public class DynamoDBMetadataStore implements MetadataStore {
itemCount++;
if (deletionBatch.size() == S3GUARD_DDB_BATCH_WRITE_REQUEST_LIMIT) {
Thread.sleep(delay);
- processBatchWriteRequest(pathToKey(deletionBatch), new Item[0]);
+ processBatchWriteRequest(pathToKey(deletionBatch), null);
deletionBatch.clear();
}
}
if (deletionBatch.size() > 0) {
Thread.sleep(delay);
- processBatchWriteRequest(pathToKey(deletionBatch), new Item[0]);
+ processBatchWriteRequest(pathToKey(deletionBatch), null);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/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 52e5b2a..d7e256d 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
@@ -101,29 +101,33 @@ public class LocalMetadataStore implements MetadataStore {
}
@Override
- public void delete(Path path) throws IOException {
- doDelete(path, false);
+ public void delete(Path p) throws IOException {
+ doDelete(p, false, true);
+ }
+
+ @Override
+ public void forgetMetadata(Path p) throws IOException {
+ doDelete(p, false, false);
}
@Override
public void deleteSubtree(Path path) throws IOException {
- doDelete(path, true);
+ doDelete(path, true, true);
}
- private synchronized void doDelete(Path p, boolean recursive) {
+ private synchronized void doDelete(Path p, boolean recursive, boolean
+ tombstone) {
Path path = standardize(p);
- // We could implement positive hit for 'deleted' files. For now we
- // do not track them.
// Delete entry from file cache, then from cached parent directory, if any
- removeHashEntries(path);
+ deleteHashEntries(path, tombstone);
if (recursive) {
// Remove all entries that have this dir as path prefix.
- clearHashByAncestor(path, dirHash);
- clearHashByAncestor(path, fileHash);
+ deleteHashByAncestor(path, dirHash, tombstone);
+ deleteHashByAncestor(path, fileHash, tombstone);
}
}
@@ -157,7 +161,7 @@ public class LocalMetadataStore implements MetadataStore {
*/
private Tristate isEmptyDirectory(Path p) {
DirListingMetadata dirMeta = dirHash.get(p);
- return dirMeta.isEmpty();
+ return dirMeta.withoutTombstones().isEmpty();
}
@Override
@@ -186,9 +190,9 @@ public class LocalMetadataStore implements MetadataStore {
synchronized (this) {
// 1. Delete pathsToDelete
- for (Path p : pathsToDelete) {
- LOG.debug("move: deleting metadata {}", p);
- delete(p);
+ for (Path meta : pathsToDelete) {
+ LOG.debug("move: deleting metadata {}", meta);
+ delete(meta);
}
// 2. Create new destination path metadata
@@ -323,13 +327,25 @@ public class LocalMetadataStore implements MetadataStore {
}
@VisibleForTesting
- static <T> void clearHashByAncestor(Path ancestor, Map<Path, T> hash) {
+ static <T> void deleteHashByAncestor(Path ancestor, Map<Path, T> hash,
+ boolean tombstone) {
for (Iterator<Map.Entry<Path, T>> it = hash.entrySet().iterator();
it.hasNext();) {
Map.Entry<Path, T> entry = it.next();
Path f = entry.getKey();
+ T meta = entry.getValue();
if (isAncestorOf(ancestor, f)) {
- it.remove();
+ if (tombstone) {
+ if (meta instanceof PathMetadata) {
+ entry.setValue((T) PathMetadata.tombstone(f));
+ } else if (meta instanceof DirListingMetadata) {
+ it.remove();
+ } else {
+ throw new IllegalStateException("Unknown type in hash");
+ }
+ } else {
+ it.remove();
+ }
}
}
}
@@ -351,16 +367,21 @@ public class LocalMetadataStore implements MetadataStore {
* Update fileHash and dirHash to reflect deletion of file 'f'. Call with
* lock held.
*/
- private void removeHashEntries(Path path) {
+ private void deleteHashEntries(Path path, boolean tombstone) {
// Remove target file/dir
LOG.debug("delete file entry for {}", path);
- fileHash.remove(path);
+ if (tombstone) {
+ fileHash.put(path, PathMetadata.tombstone(path));
+ } else {
+ fileHash.remove(path);
+ }
// Update this and parent dir listing, if any
/* If this path is a dir, remove its listing */
LOG.debug("removing listing of {}", path);
+
dirHash.remove(path);
/* Remove this path from parent's dir listing */
@@ -369,7 +390,11 @@ public class LocalMetadataStore implements MetadataStore {
DirListingMetadata dir = dirHash.get(parent);
if (dir != null) {
LOG.debug("removing parent's entry for {} ", path);
- dir.remove(path);
+ if (tombstone) {
+ dir.markDeleted(path);
+ } else {
+ dir.remove(path);
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/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 5511532..9502a8a 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
@@ -22,6 +22,7 @@ import java.io.Closeable;
import java.io.IOException;
import java.util.Collection;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
@@ -55,7 +56,8 @@ public interface MetadataStore extends Closeable {
void initialize(Configuration conf) throws IOException;
/**
- * Deletes exactly one path.
+ * Deletes exactly one path, leaving a tombstone to prevent lingering,
+ * inconsistent copies of it from being listed.
*
* @param path the path to delete
* @throws IOException if there is an error
@@ -63,7 +65,20 @@ public interface MetadataStore extends Closeable {
void delete(Path path) throws IOException;
/**
- * Deletes the entire sub-tree rooted at the given path.
+ * Removes the record of exactly one path. Does not leave a tombstone (see
+ * {@link MetadataStore#delete(Path)}. It is currently intended for testing
+ * only, and a need to use it as part of normal FileSystem usage is not
+ * anticipated.
+ *
+ * @param path the path to delete
+ * @throws IOException if there is an error
+ */
+ @VisibleForTesting
+ void forgetMetadata(Path path) throws IOException;
+
+ /**
+ * Deletes the entire sub-tree rooted at the given path, leaving tombstones
+ * to prevent lingering, inconsistent copies of it from being listed.
*
* In addition to affecting future calls to {@link #get(Path)},
* implementations must also update any stored {@code DirListingMetadata}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
new file mode 100644
index 0000000..272b1f4
--- /dev/null
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a.s3guard;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Queue;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * {@code MetadataStoreListFilesIterator} is a {@link RemoteIterator} that
+ * is similar to {@code DescendantsIterator} but does not return directories
+ * that have (or may have) children, and will also provide access to the set of
+ * tombstones to allow recently deleted S3 objects to be filtered out from a
+ * corresponding request. In other words, it returns tombstones and the same
+ * set of objects that should exist in S3: empty directories, and files, and not
+ * other directories whose existence is inferred therefrom.
+ *
+ * For example, assume the consistent store contains metadata representing this
+ * file system structure:
+ *
+ * <pre>
+ * /dir1
+ * |-- dir2
+ * | |-- file1
+ * | `-- file2
+ * `-- dir3
+ * |-- dir4
+ * | `-- file3
+ * |-- dir5
+ * | `-- file4
+ * `-- dir6
+ * </pre>
+ *
+ * Consider this code sample:
+ * <pre>
+ * final PathMetadata dir1 = get(new Path("/dir1"));
+ * for (MetadataStoreListFilesIterator files =
+ * new MetadataStoreListFilesIterator(dir1); files.hasNext(); ) {
+ * final FileStatus status = files.next().getFileStatus();
+ * System.out.printf("%s %s%n", status.isDirectory() ? 'D' : 'F',
+ * status.getPath());
+ * }
+ * </pre>
+ *
+ * The output is:
+ * <pre>
+ * F /dir1/dir2/file1
+ * F /dir1/dir2/file2
+ * F /dir1/dir3/dir4/file3
+ * F /dir1/dir3/dir5/file4
+ * D /dir1/dir3/dir6
+ * </pre>
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public class MetadataStoreListFilesIterator implements
+ RemoteIterator<FileStatus> {
+ public static final Logger LOG = LoggerFactory.getLogger(
+ MetadataStoreListFilesIterator.class);
+
+ private final boolean allowAuthoritative;
+ private final MetadataStore metadataStore;
+ private final Set<Path> tombstones = new HashSet<>();
+ private Iterator<FileStatus> leafNodesIterator = null;
+
+ public MetadataStoreListFilesIterator(MetadataStore ms, PathMetadata meta,
+ boolean allowAuthoritative) throws IOException {
+ Preconditions.checkNotNull(ms);
+ this.metadataStore = ms;
+ this.allowAuthoritative = allowAuthoritative;
+ prefetch(meta);
+ }
+
+ private void prefetch(PathMetadata meta) throws IOException {
+ final Queue<PathMetadata> queue = new LinkedList<>();
+ final Collection<FileStatus> leafNodes = new ArrayList<>();
+
+ if (meta != null) {
+ final Path path = meta.getFileStatus().getPath();
+ if (path.isRoot()) {
+ DirListingMetadata rootListing = metadataStore.listChildren(path);
+ if (rootListing != null) {
+ tombstones.addAll(rootListing.listTombstones());
+ queue.addAll(rootListing.withoutTombstones().getListing());
+ }
+ } else {
+ queue.add(meta);
+ }
+ }
+
+ while(!queue.isEmpty()) {
+ PathMetadata nextMetadata = queue.poll();
+ FileStatus nextStatus = nextMetadata.getFileStatus();
+ if (nextStatus.isFile()) {
+ // All files are leaf nodes by definition
+ leafNodes.add(nextStatus);
+ continue;
+ }
+ if (nextStatus.isDirectory()) {
+ final Path path = nextStatus.getPath();
+ DirListingMetadata children = metadataStore.listChildren(path);
+ if (children != null) {
+ tombstones.addAll(children.listTombstones());
+ Collection<PathMetadata> liveChildren =
+ children.withoutTombstones().getListing();
+ if (!liveChildren.isEmpty()) {
+ // If it's a directory, has children, not all deleted, then we
+ // add the children to the queue and move on to the next node
+ queue.addAll(liveChildren);
+ continue;
+ } else if (allowAuthoritative && children.isAuthoritative()) {
+ leafNodes.add(nextStatus);
+ }
+ }
+ }
+ // Directories that *might* be empty are ignored for now, since we
+ // cannot confirm that they are empty without incurring other costs.
+ // Users of this class can still discover empty directories via S3's
+ // fake directories, subject to the same consistency semantics as before.
+ // The only other possibility is a symlink, which is unsupported on S3A.
+ }
+ leafNodesIterator = leafNodes.iterator();
+ }
+
+ @Override
+ public boolean hasNext() {
+ return leafNodesIterator.hasNext();
+ }
+
+ @Override
+ public FileStatus next() {
+ return leafNodesIterator.next();
+ }
+
+ public Set<Path> listTombstones() {
+ return tombstones;
+ }
+}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
index 3869d13..65019eb 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
@@ -52,6 +52,11 @@ public class NullMetadataStore implements MetadataStore {
}
@Override
+ public void forgetMetadata(Path path) throws IOException {
+ return;
+ }
+
+ @Override
public void deleteSubtree(Path path) throws IOException {
return;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
index b5d4f04..d799e16 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
@@ -22,6 +22,7 @@ import com.google.common.base.Preconditions;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.Tristate;
/**
@@ -34,6 +35,13 @@ public class PathMetadata {
private final FileStatus fileStatus;
private Tristate isEmptyDirectory;
+ private boolean isDeleted;
+
+ public static PathMetadata tombstone(Path path) {
+ long now = System.currentTimeMillis();
+ FileStatus status = new FileStatus(0, false, 0, 0, now, path);
+ return new PathMetadata(status, Tristate.UNKNOWN, true);
+ }
/**
* Creates a new {@code PathMetadata} containing given {@code FileStatus}.
@@ -44,6 +52,11 @@ public class PathMetadata {
}
public PathMetadata(FileStatus fileStatus, Tristate isEmptyDir) {
+ this(fileStatus, isEmptyDir, false);
+ }
+
+ public PathMetadata(FileStatus fileStatus, Tristate isEmptyDir, boolean
+ isDeleted) {
Preconditions.checkNotNull(fileStatus, "fileStatus must be non-null");
Preconditions.checkNotNull(fileStatus.getPath(), "fileStatus path must be" +
" non-null");
@@ -51,6 +64,7 @@ public class PathMetadata {
" be absolute");
this.fileStatus = fileStatus;
this.isEmptyDirectory = isEmptyDir;
+ this.isDeleted = isDeleted;
}
/**
@@ -73,6 +87,14 @@ public class PathMetadata {
this.isEmptyDirectory = isEmptyDirectory;
}
+ public boolean isDeleted() {
+ return isDeleted;
+ }
+
+ void setIsDeleted(boolean isDeleted) {
+ this.isDeleted = isDeleted;
+ }
+
@Override
public boolean equals(Object o) {
if (!(o instanceof PathMetadata)) {
@@ -91,6 +113,7 @@ public class PathMetadata {
return "PathMetadata{" +
"fileStatus=" + fileStatus +
"; isEmptyDirectory=" + isEmptyDirectory +
+ "; isDeleted=" + isDeleted +
'}';
}
@@ -99,10 +122,10 @@ public class PathMetadata {
* @param sb target StringBuilder
*/
public void prettyPrint(StringBuilder sb) {
- sb.append(String.format("%-5s %-20s %-7d %s",
+ sb.append(String.format("%-5s %-20s %-7d %-8s %-6s",
fileStatus.isDirectory() ? "dir" : "file",
fileStatus.getPath().toString(), fileStatus.getLen(),
- isEmptyDirectory.name()));
+ isEmptyDirectory.name(), isDeleted));
sb.append(fileStatus);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
index 1c04016..c206a00 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
@@ -39,6 +39,7 @@ import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.Constants;
+import org.apache.hadoop.fs.s3a.Tristate;
/**
* Defines methods for translating between domain model objects and their
@@ -62,6 +63,7 @@ final class PathMetadataDynamoDBTranslation {
static final String FILE_LENGTH = "file_length";
@VisibleForTesting
static final String BLOCK_SIZE = "block_size";
+ static final String IS_DELETED = "is_deleted";
/** Table version field {@value} in version marker item. */
@VisibleForTesting
@@ -133,8 +135,10 @@ final class PathMetadataDynamoDBTranslation {
fileStatus = new FileStatus(len, false, 1, block, modTime, 0, null,
username, username, path);
}
+ boolean isDeleted =
+ item.hasAttribute(IS_DELETED) ? item.getBoolean(IS_DELETED) : false;
- return new PathMetadata(fileStatus);
+ return new PathMetadata(fileStatus, Tristate.UNKNOWN, isDeleted);
}
/**
@@ -154,6 +158,7 @@ final class PathMetadataDynamoDBTranslation {
.withLong(MOD_TIME, status.getModificationTime())
.withLong(BLOCK_SIZE, status.getBlockSize());
}
+ item.withBoolean(IS_DELETED, meta.isDeleted());
return item;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
index 53dc991..19f0e50 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
@@ -39,6 +39,7 @@ import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Set;
import static org.apache.hadoop.fs.s3a.Constants.*;
import static org.apache.hadoop.fs.s3a.Statistic.*;
@@ -143,7 +144,8 @@ public final class S3Guard {
/**
* Convert the data of a directory listing to an array of {@link FileStatus}
- * entries. If the listing is null an empty array is returned.
+ * entries. Tombstones are filtered out at this point. If the listing is null
+ * an empty array is returned.
* @param dirMeta directory listing -may be null
* @return a possibly-empty array of file status entries
*/
@@ -153,15 +155,15 @@ public final class S3Guard {
}
Collection<PathMetadata> listing = dirMeta.getListing();
- FileStatus[] statuses = new FileStatus[listing.size()];
+ List<FileStatus> statuses = new ArrayList<>();
- // HADOOP-13760: filter out deleted entries here
- int i = 0;
for (PathMetadata pm : listing) {
- statuses[i++] = pm.getFileStatus();
+ if (!pm.isDeleted()) {
+ statuses.add(pm.getFileStatus());
+ }
}
- return statuses;
+ return statuses.toArray(new FileStatus[0]);
}
/**
@@ -196,6 +198,8 @@ public final class S3Guard {
false);
}
+ Set<Path> deleted = dirMeta.listTombstones();
+
// Since we treat the MetadataStore as a "fresher" or "consistent" view
// of metadata, we always use its metadata first.
@@ -203,10 +207,11 @@ public final class S3Guard {
// we will basically start with the set of directory entries in the
// DirListingMetadata, and add any that only exist in the backingStatuses.
- // HADOOP-13760: filter out deleted files via PathMetadata#isDeleted() here
-
boolean changed = false;
for (FileStatus s : backingStatuses) {
+ if (deleted.contains(s.getPath())) {
+ continue;
+ }
// Minor race condition here. Multiple threads could add to this
// mutable DirListingMetadata. Since it is backed by a
@@ -245,7 +250,7 @@ public final class S3Guard {
* list will contain [/a/b/c/d, /a/b/c, /a/b]. /a/b/c/d is
* an empty, dir, and the other dirs only contain their child
* dir.
- * @param owner Hadoop user name
+ * @param owner Hadoop user name.
*/
public static void makeDirsOrdered(MetadataStore ms, List<Path> dirs,
String owner) {
@@ -280,7 +285,8 @@ public final class S3Guard {
children = new ArrayList<>(1);
children.add(new PathMetadata(prevStatus));
}
- DirListingMetadata dirMeta = new DirListingMetadata(f, children, true);
+ DirListingMetadata dirMeta =
+ new DirListingMetadata(f, children, true);
try {
ms.put(dirMeta);
ms.put(new PathMetadata(status));
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
index 3d4f11d..9bd0cb8 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
@@ -446,7 +446,7 @@ public abstract class S3GuardTool extends Configured implements Tool {
private long importDir(FileStatus status) throws IOException {
Preconditions.checkArgument(status.isDirectory());
RemoteIterator<LocatedFileStatus> it =
- s3a.listFilesAndDirectories(status.getPath(), true);
+ s3a.listFilesAndEmptyDirectories(status.getPath(), true);
long items = 0;
while (it.hasNext()) {
@@ -686,7 +686,8 @@ public abstract class S3GuardTool extends Configured implements Tool {
} catch (FileNotFoundException e) {
}
PathMetadata meta = ms.get(qualified);
- FileStatus msStatus = meta != null ? meta.getFileStatus() : null;
+ FileStatus msStatus = (meta != null && !meta.isDeleted()) ?
+ meta.getFileStatus() : null;
compareDir(msStatus, s3Status, out);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
index 55e310b..fb6e370 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
@@ -39,45 +39,47 @@ public class ITestS3GuardEmptyDirs extends AbstractS3ATestBase {
@Test
public void testEmptyDirs() throws Exception {
-
S3AFileSystem fs = getFileSystem();
Assume.assumeTrue(fs.hasMetadataStore());
MetadataStore configuredMs = fs.getMetadataStore();
-
- // 1. Simulate files already existing in the bucket before we started our
- // cluster. Temporarily disable the MetadataStore so it doesn't witness
- // us creating these files.
-
- fs.setMetadataStore(new NullMetadataStore());
-
Path existingDir = path("existing-dir");
- assertTrue(fs.mkdirs(existingDir));
Path existingFile = path("existing-dir/existing-file");
- touch(fs, existingFile);
+ try {
+ // 1. Simulate files already existing in the bucket before we started our
+ // cluster. Temporarily disable the MetadataStore so it doesn't witness
+ // us creating these files.
+
+ fs.setMetadataStore(new NullMetadataStore());
+ assertTrue(fs.mkdirs(existingDir));
+ touch(fs, existingFile);
- // 2. Simulate (from MetadataStore's perspective) starting our cluster and
- // creating a file in an existing directory.
- fs.setMetadataStore(configuredMs); // "start cluster"
- Path newFile = path("existing-dir/new-file");
- touch(fs, newFile);
+ // 2. Simulate (from MetadataStore's perspective) starting our cluster and
+ // creating a file in an existing directory.
+ fs.setMetadataStore(configuredMs); // "start cluster"
+ Path newFile = path("existing-dir/new-file");
+ touch(fs, newFile);
- S3AFileStatus status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should not be empty dir", Tristate.FALSE,
- status.isEmptyDirectory());
+ S3AFileStatus status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should not be empty dir", Tristate.FALSE,
+ status.isEmptyDirectory());
- // 3. Assert that removing the only file the MetadataStore witnessed
- // being created doesn't cause it to think the directory is now empty.
- fs.delete(newFile, false);
- status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should not be empty dir", Tristate.FALSE,
- status.isEmptyDirectory());
+ // 3. Assert that removing the only file the MetadataStore witnessed
+ // being created doesn't cause it to think the directory is now empty.
+ fs.delete(newFile, false);
+ status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should not be empty dir", Tristate.FALSE,
+ status.isEmptyDirectory());
- // 4. Assert that removing the final file, that existed "before"
- // MetadataStore started, *does* cause the directory to be marked empty.
- fs.delete(existingFile, false);
- status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should be empty dir now", Tristate.TRUE,
- status.isEmptyDirectory());
+ // 4. Assert that removing the final file, that existed "before"
+ // MetadataStore started, *does* cause the directory to be marked empty.
+ fs.delete(existingFile, false);
+ status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should be empty dir now", Tristate.TRUE,
+ status.isEmptyDirectory());
+ } finally {
+ configuredMs.forgetMetadata(existingFile);
+ configuredMs.forgetMetadata(existingDir);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
index 5e83906..8771fd2 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
@@ -56,8 +56,185 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
return new S3AContract(conf);
}
+ /**
+ * Helper function for other test cases: does a single rename operation and
+ * validates the aftermath.
+ * @param mkdirs Directories to create
+ * @param srcdirs Source paths for rename operation
+ * @param dstdirs Destination paths for rename operation
+ * @param yesdirs Files that must exist post-rename (e.g. srcdirs children)
+ * @param nodirs Files that must not exist post-rename (e.g. dstdirs children)
+ * @throws Exception
+ */
+ private void doTestRenameSequence(Path[] mkdirs, Path[] srcdirs,
+ Path[] dstdirs, Path[] yesdirs, Path[] nodirs) throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Assume.assumeTrue(fs.hasMetadataStore());
+
+ if (mkdirs != null) {
+ for (Path mkdir : mkdirs) {
+ assertTrue(fs.mkdirs(mkdir));
+ }
+ Thread.sleep(InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
+ }
+
+ assertTrue("srcdirs and dstdirs must have equal length",
+ srcdirs.length == dstdirs.length);
+ for (int i = 0; i < srcdirs.length; i++) {
+ assertTrue("Rename returned false: " + srcdirs[i] + " -> " + dstdirs[i],
+ fs.rename(srcdirs[i], dstdirs[i]));
+ }
+
+ for (Path yesdir : yesdirs) {
+ assertTrue("Path was supposed to exist: " + yesdir, fs.exists(yesdir));
+ }
+ for (Path nodir : nodirs) {
+ assertFalse("Path is not supposed to exist: " + nodir, fs.exists(nodir));
+ }
+ }
+
+ /**
+ * Tests that after renaming a directory, the original directory and its
+ * contents are indeed missing and the corresponding new paths are visible.
+ * @throws Exception
+ */
+ @Test
+ public void testConsistentListAfterRename() throws Exception {
+ Path[] mkdirs = {
+ path("d1/f"),
+ path("d1/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)
+ };
+ Path[] srcdirs = {path("d1")};
+ Path[] dstdirs = {path("d2")};
+ Path[] yesdirs = {path("d2"), path("d2/f"),
+ path("d2/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)};
+ Path[] nodirs = {path("d1"), path("d1/f"),
+ path("d1/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)};
+ doTestRenameSequence(mkdirs, srcdirs, dstdirs, yesdirs, nodirs);
+ getFileSystem().delete(path("d1"), true);
+ getFileSystem().delete(path("d2"), true);
+ }
+
+ /**
+ * Tests a circular sequence of renames to verify that overwriting recently
+ * deleted files and reading recently created files from rename operations
+ * works as expected.
+ * @throws Exception
+ */
+ @Test
+ public void testRollingRenames() throws Exception {
+ Path[] dir0 = {path("rolling/1")};
+ Path[] dir1 = {path("rolling/2")};
+ Path[] dir2 = {path("rolling/3")};
+ // These sets have to be in reverse order compared to the movement
+ Path[] setA = {dir1[0], dir0[0]};
+ Path[] setB = {dir2[0], dir1[0]};
+ Path[] setC = {dir0[0], dir2[0]};
+
+ for(int i = 0; i < 2; i++) {
+ Path[] firstSet = i == 0 ? setA : null;
+ doTestRenameSequence(firstSet, setA, setB, setB, dir0);
+ doTestRenameSequence(null, setB, setC, setC, dir1);
+ doTestRenameSequence(null, setC, setA, setA, dir2);
+ }
+
+ S3AFileSystem fs = getFileSystem();
+ assertFalse("Renaming deleted file should have failed",
+ fs.rename(dir2[0], dir1[0]));
+ assertTrue("Renaming over existing file should have succeeded",
+ fs.rename(dir1[0], dir0[0]));
+ }
+
+ /**
+ * Tests that deleted files immediately stop manifesting in list operations
+ * even when the effect in S3 is delayed.
+ * @throws Exception
+ */
+ @Test
+ public void testConsistentListAfterDelete() throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ // test will fail if NullMetadataStore (the default) is configured: skip it.
+ Assume.assumeTrue(fs.hasMetadataStore());
+
+ // Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
+ // in listObjects() results via InconsistentS3Client
+ Path inconsistentPath =
+ path("a/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING);
+
+ Path[] testDirs = {path("a/b/dir1"),
+ path("a/b/dir2"),
+ inconsistentPath};
+
+ for (Path path : testDirs) {
+ assertTrue(fs.mkdirs(path));
+ }
+ Thread.sleep(2 * InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
+ for (Path path : testDirs) {
+ assertTrue(fs.delete(path, false));
+ }
+
+ FileStatus[] paths = fs.listStatus(path("a/b/"));
+ List<Path> list = new ArrayList<>();
+ for (FileStatus fileState : paths) {
+ list.add(fileState.getPath());
+ }
+ assertFalse(list.contains(path("a/b/dir1")));
+ assertFalse(list.contains(path("a/b/dir2")));
+ // This should fail without S3Guard, and succeed with it.
+ assertFalse(list.contains(inconsistentPath));
+ }
+
+ /**
+ * Tests that rename immediately after files in the source directory are
+ * deleted results in exactly the correct set of destination files and none
+ * of the source files.
+ * @throws Exception
+ */
@Test
- public void testConsistentListStatus() throws Exception {
+ public void testConsistentRenameAfterDelete() throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ // test will fail if NullMetadataStore (the default) is configured: skip it.
+ Assume.assumeTrue(fs.hasMetadataStore());
+
+ // Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
+ // in listObjects() results via InconsistentS3Client
+ Path inconsistentPath =
+ path("a/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING);
+
+ Path[] testDirs = {path("a/b/dir1"),
+ path("a/b/dir2"),
+ inconsistentPath};
+
+ for (Path path : testDirs) {
+ assertTrue(fs.mkdirs(path));
+ }
+ Thread.sleep(2 * InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
+ assertTrue(fs.delete(testDirs[1], false));
+ assertTrue(fs.delete(testDirs[2], false));
+
+ fs.rename(path("a"), path("a3"));
+ FileStatus[] paths = fs.listStatus(path("a3/b"));
+ List<Path> list = new ArrayList<>();
+ for (FileStatus fileState : paths) {
+ list.add(fileState.getPath());
+ }
+ assertTrue(list.contains(path("a3/b/dir1")));
+ assertFalse(list.contains(path("a3/b/dir2")));
+ // This should fail without S3Guard, and succeed with it.
+ assertFalse(list.contains(
+ path("a3/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)));
+
+ try {
+ RemoteIterator<LocatedFileStatus> old = fs.listFilesAndEmptyDirectories(
+ path("a"), true);
+ fail("Recently renamed dir should not be visible");
+ } catch(FileNotFoundException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void testConsistentListStatusAfterPut() throws Exception {
S3AFileSystem fs = getFileSystem();
@@ -90,23 +267,25 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
}
/**
- * Similar to {@link #testConsistentListStatus()}, this tests that the FS
- * listLocatedStatus() call will return consistent list.
+ * Similar to {@link #testConsistentListStatusAfterPut()}, this tests that the
+ * FS listLocatedStatus() call will return consistent list.
*/
@Test
- public void testConsistentListLocatedStatus() throws Exception {
+ public void testConsistentListLocatedStatusAfterPut() throws Exception {
final S3AFileSystem fs = getFileSystem();
// This test will fail if NullMetadataStore (the default) is configured:
// skip it.
Assume.assumeTrue(fs.hasMetadataStore());
- fs.mkdirs(path("doTestConsistentListLocatedStatus"));
+ String rootDir = "doTestConsistentListLocatedStatusAfterPut";
+ fs.mkdirs(path(rootDir));
final int[] numOfPaths = {0, 1, 10};
for (int normalPathNum : numOfPaths) {
for (int delayedPathNum : numOfPaths) {
LOG.info("Testing with normalPathNum={}, delayedPathNum={}",
normalPathNum, delayedPathNum);
- doTestConsistentListLocatedStatus(fs, normalPathNum, delayedPathNum);
+ doTestConsistentListLocatedStatusAfterPut(fs, rootDir, normalPathNum,
+ delayedPathNum);
}
}
}
@@ -118,18 +297,18 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
* @param delayedPathNum number paths listed with delaying
* @throws Exception
*/
- private void doTestConsistentListLocatedStatus(S3AFileSystem fs,
- int normalPathNum, int delayedPathNum) throws Exception {
+ private void doTestConsistentListLocatedStatusAfterPut(S3AFileSystem fs,
+ String rootDir, int normalPathNum, int delayedPathNum) throws Exception {
final List<Path> testDirs = new ArrayList<>(normalPathNum + delayedPathNum);
int index = 0;
for (; index < normalPathNum; index++) {
- testDirs.add(path("doTestConsistentListLocatedStatus/dir-" + index));
+ testDirs.add(path(rootDir + "/dir-" +
+ index));
}
for (; index < normalPathNum + delayedPathNum; index++) {
// Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
// in listObjects() results via InconsistentS3Client
- testDirs.add(path("doTestConsistentListLocatedStatus/dir-" + index
- + DELAY_KEY_SUBSTRING));
+ testDirs.add(path(rootDir + "/dir-" + index + DELAY_KEY_SUBSTRING));
}
for (Path path : testDirs) {
@@ -141,7 +320,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
// this should return the union data from S3 and MetadataStore
final RemoteIterator<LocatedFileStatus> statusIterator =
- fs.listLocatedStatus(path("doTestConsistentListLocatedStatus/"));
+ fs.listLocatedStatus(path(rootDir + "/"));
List<Path> list = new ArrayList<>();
for (; statusIterator.hasNext();) {
list.add(statusIterator.next().getPath());
@@ -223,10 +402,13 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
fileNames.add("file-" + index + "-" + DELAY_KEY_SUBSTRING);
}
+ int filesAndEmptyDirectories = 0;
+
// create files under each test directory
for (Path dir : testDirs) {
for (String fileName : fileNames) {
writeTextFile(fs, new Path(dir, fileName), "I, " + fileName, false);
+ filesAndEmptyDirectories++;
}
}
@@ -251,7 +433,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
verifyFileIsListed(listedFiles, baseTestDir, fileNames);
} else {
assertEquals("Unexpected number of files returned by listFiles() call",
- testDirs.size() * (normalFileNum + delayedFileNum),
+ filesAndEmptyDirectories,
listedFiles.size());
for (Path dir : testDirs) {
verifyFileIsListed(listedFiles, dir, fileNames);
@@ -363,6 +545,11 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
assertEquals("Unexpected number of results from metadata store. "
+ "Should have /OnS3 and /OnS3AndMS: " + mdResults,
2, mdResults.numEntries());
+
+ // If we don't clean this up, the next test run will fail because it will
+ // have recorded /OnS3 being deleted even after it's written to noS3Guard.
+ getFileSystem().getMetadataStore().forgetMetadata(
+ new Path(directory, "OnS3"));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[3/6] hadoop git commit: HADOOP-13760. S3Guard: add delete tracking.
Posted by ma...@apache.org.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
new file mode 100644
index 0000000..43eb2c0
--- /dev/null
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+/**
+ * Place for the S3A listing classes; keeps all the small classes under control.
+ */
+public class TestListing extends AbstractS3AMockTest {
+
+ private static class MockRemoteIterator<FileStatus> implements
+ RemoteIterator<FileStatus> {
+ private Iterator<FileStatus> iterator;
+
+ MockRemoteIterator(Collection<FileStatus> source) {
+ iterator = source.iterator();
+ }
+
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
+
+ public FileStatus next() {
+ return iterator.next();
+ }
+ }
+
+ private FileStatus blankFileStatus(Path path) {
+ return new FileStatus(0, true, 0, 0, 0, path);
+ }
+
+ @Test
+ public void testTombstoneReconcilingIterator() throws Exception {
+ Path parent = new Path("/parent");
+ Path liveChild = new Path(parent, "/liveChild");
+ Path deletedChild = new Path(parent, "/deletedChild");
+ Path[] allFiles = {parent, liveChild, deletedChild};
+ Path[] liveFiles = {parent, liveChild};
+
+ Listing listing = new Listing(fs);
+ Collection<FileStatus> statuses = new ArrayList<>();
+ statuses.add(blankFileStatus(parent));
+ statuses.add(blankFileStatus(liveChild));
+ statuses.add(blankFileStatus(deletedChild));
+
+ Set<Path> tombstones = new HashSet<>();
+ tombstones.add(deletedChild);
+
+ RemoteIterator<FileStatus> sourceIterator = new MockRemoteIterator(
+ statuses);
+ RemoteIterator<LocatedFileStatus> locatedIterator =
+ listing.createLocatedFileStatusIterator(sourceIterator);
+ RemoteIterator<LocatedFileStatus> reconcilingIterator =
+ listing.createTombstoneReconcilingIterator(locatedIterator, tombstones);
+
+ Set<Path> expectedPaths = new HashSet<>();
+ expectedPaths.add(parent);
+ expectedPaths.add(liveChild);
+
+ Set<Path> actualPaths = new HashSet<>();
+ while (reconcilingIterator.hasNext()) {
+ actualPaths.add(reconcilingIterator.next().getPath());
+ }
+ Assert.assertTrue(actualPaths.equals(expectedPaths));
+ }
+}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/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 99acf6e..dfa8a9e 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
@@ -21,6 +21,7 @@ package org.apache.hadoop.fs.s3a.s3guard;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.fs.s3a.S3ATestUtils;
import org.apache.hadoop.fs.s3a.Tristate;
@@ -134,12 +135,52 @@ public abstract class MetadataStoreTestBase extends Assert {
}
/**
+ * Helper function for verifying DescendantsIterator and
+ * MetadataStoreListFilesIterator behavior.
+ * @param createNodes List of paths to create
+ * @param checkNodes List of paths that the iterator should return
+ * @throws IOException
+ */
+ private void doTestDescendantsIterator(
+ Class implementation, String[] createNodes,
+ String[] checkNodes) throws Exception {
+ // we set up the example file system tree in metadata store
+ for (String pathStr : createNodes) {
+ final FileStatus status = pathStr.contains("file")
+ ? basicFileStatus(strToPath(pathStr), 100, false)
+ : basicFileStatus(strToPath(pathStr), 0, true);
+ ms.put(new PathMetadata(status));
+ }
+
+ final PathMetadata rootMeta = new PathMetadata(makeDirStatus("/"));
+ RemoteIterator<FileStatus> iterator;
+ if (implementation == DescendantsIterator.class) {
+ iterator = new DescendantsIterator(ms, rootMeta);
+ } else if (implementation == MetadataStoreListFilesIterator.class) {
+ iterator = new MetadataStoreListFilesIterator(ms, rootMeta, false);
+ } else {
+ throw new UnsupportedOperationException("Unrecognized class");
+ }
+
+ final Set<String> actual = new HashSet<>();
+ while (iterator.hasNext()) {
+ final Path p = iterator.next().getPath();
+ actual.add(Path.getPathWithoutSchemeAndAuthority(p).toString());
+ }
+ LOG.info("We got {} by iterating DescendantsIterator", actual);
+
+ if (!allowMissing()) {
+ assertEquals(Sets.newHashSet(checkNodes), actual);
+ }
+ }
+
+ /**
* Test that we can get the whole sub-tree by iterating DescendantsIterator.
*
* The tree is similar to or same as the example in code comment.
*/
@Test
- public void testDescendantsIterator() throws IOException {
+ public void testDescendantsIterator() throws Exception {
final String[] tree = new String[] {
"/dir1",
"/dir1/dir2",
@@ -152,26 +193,38 @@ public abstract class MetadataStoreTestBase extends Assert {
"/dir1/dir3/dir5/file4",
"/dir1/dir3/dir6"
};
- // we set up the example file system tree in metadata store
- for (String pathStr : tree) {
- final FileStatus status = pathStr.contains("file")
- ? basicFileStatus(strToPath(pathStr), 100, false)
- : basicFileStatus(strToPath(pathStr), 0, true);
- ms.put(new PathMetadata(status));
- }
-
- final Set<String> actual = new HashSet<>();
- final PathMetadata rootMeta = new PathMetadata(makeDirStatus("/"));
- for (DescendantsIterator desc = new DescendantsIterator(ms, rootMeta);
- desc.hasNext();) {
- final Path p = desc.next().getPath();
- actual.add(Path.getPathWithoutSchemeAndAuthority(p).toString());
- }
- LOG.info("We got {} by iterating DescendantsIterator", actual);
+ doTestDescendantsIterator(DescendantsIterator.class,
+ tree, tree);
+ }
- if (!allowMissing()) {
- assertEquals(Sets.newHashSet(tree), actual);
- }
+ /**
+ * Test that we can get the correct subset of the tree with
+ * MetadataStoreListFilesIterator.
+ *
+ * The tree is similar to or same as the example in code comment.
+ */
+ @Test
+ public void testMetadataStoreListFilesIterator() throws Exception {
+ final String[] wholeTree = new String[] {
+ "/dir1",
+ "/dir1/dir2",
+ "/dir1/dir3",
+ "/dir1/dir2/file1",
+ "/dir1/dir2/file2",
+ "/dir1/dir3/dir4",
+ "/dir1/dir3/dir5",
+ "/dir1/dir3/dir4/file3",
+ "/dir1/dir3/dir5/file4",
+ "/dir1/dir3/dir6"
+ };
+ final String[] leafNodes = new String[] {
+ "/dir1/dir2/file1",
+ "/dir1/dir2/file2",
+ "/dir1/dir3/dir4/file3",
+ "/dir1/dir3/dir5/file4"
+ };
+ doTestDescendantsIterator(MetadataStoreListFilesIterator.class, wholeTree,
+ leafNodes);
}
@Test
@@ -258,7 +311,7 @@ public abstract class MetadataStoreTestBase extends Assert {
/* Ensure delete happened. */
assertDirectorySize("/ADirectory1/db1", 1);
PathMetadata meta = ms.get(strToPath("/ADirectory1/db1/file2"));
- assertNull("File deleted", meta);
+ assertTrue("File deleted", meta == null || meta.isDeleted());
}
@Test
@@ -284,10 +337,10 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.deleteSubtree(strToPath(p + "/ADirectory1/db1/"));
assertEmptyDirectory(p + "/ADirectory1");
- assertNotCached(p + "/ADirectory1/db1");
- assertNotCached(p + "/ADirectory1/file1");
- assertNotCached(p + "/ADirectory1/file2");
- assertNotCached(p + "/ADirectory1/db1/dc1/dd1/deepFile");
+ assertDeleted(p + "/ADirectory1/db1");
+ assertDeleted(p + "/ADirectory1/file1");
+ assertDeleted(p + "/ADirectory1/file2");
+ assertDeleted(p + "/ADirectory1/db1/dc1/dd1/deepFile");
assertEmptyDirectory(p + "/ADirectory2");
}
@@ -302,11 +355,11 @@ public abstract class MetadataStoreTestBase extends Assert {
setUpDeleteTest();
ms.deleteSubtree(strToPath("/"));
- assertNotCached("/ADirectory1");
- assertNotCached("/ADirectory2");
- assertNotCached("/ADirectory2/db1");
- assertNotCached("/ADirectory2/db1/file1");
- assertNotCached("/ADirectory2/db1/file2");
+ assertDeleted("/ADirectory1");
+ assertDeleted("/ADirectory2");
+ assertDeleted("/ADirectory2/db1");
+ assertDeleted("/ADirectory2/db1/file1");
+ assertDeleted("/ADirectory2/db1/file2");
}
@Test
@@ -350,6 +403,12 @@ public abstract class MetadataStoreTestBase extends Assert {
verifyFileStatus(meta.getFileStatus(), 100);
}
+ if (!(ms instanceof NullMetadataStore)) {
+ ms.delete(strToPath(filePath));
+ meta = ms.get(strToPath(filePath));
+ assertTrue("Tombstone not left for deleted file", meta.isDeleted());
+ }
+
meta = ms.get(strToPath(dirPath));
if (!allowMissing() || meta != null) {
assertNotNull("Get found file (dir)", meta);
@@ -441,6 +500,7 @@ public abstract class MetadataStoreTestBase extends Assert {
dirMeta = ms.listChildren(strToPath("/a1"));
if (!allowMissing() || dirMeta != null) {
+ dirMeta = dirMeta.withoutTombstones();
assertListingsEqual(dirMeta.getListing(), "/a1/b1", "/a1/b2");
}
@@ -486,6 +546,7 @@ public abstract class MetadataStoreTestBase extends Assert {
Collection<PathMetadata> entries;
DirListingMetadata dirMeta = ms.listChildren(strToPath("/"));
if (!allowMissing() || dirMeta != null) {
+ dirMeta = dirMeta.withoutTombstones();
assertNotNull("Listing root", dirMeta);
entries = dirMeta.getListing();
assertListingsEqual(entries, "/a1", "/a2", "/a3");
@@ -513,13 +574,12 @@ public abstract class MetadataStoreTestBase extends Assert {
dirMeta = ms.listChildren(strToPath("/a1"));
if (!allowMissing() || dirMeta != null) {
assertNotNull("Listing /a1", dirMeta);
- entries = dirMeta.getListing();
+ entries = dirMeta.withoutTombstones().getListing();
assertListingsEqual(entries, "/a1/b2");
}
PathMetadata meta = ms.get(strToPath("/a1/b1/file1"));
- // TODO allow return of PathMetadata with isDeleted == true
- assertNull("Src path deleted", meta);
+ assertTrue("Src path deleted", meta == null || meta.isDeleted());
// Assert dest looks right
meta = ms.get(strToPath("/b1/file1"));
@@ -596,7 +656,7 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.prune(cutoff);
ls = ms.listChildren(strToPath("/pruneFiles"));
if (allowMissing()) {
- assertNotCached("/pruneFiles/old");
+ assertDeleted("/pruneFiles/old");
} else {
assertListingsEqual(ls.getListing(), "/pruneFiles/new");
}
@@ -625,7 +685,7 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.prune(cutoff);
- assertNotCached("/pruneDirs/dir/file");
+ assertDeleted("/pruneDirs/dir/file");
}
/*
@@ -646,6 +706,7 @@ public abstract class MetadataStoreTestBase extends Assert {
"file3"));
DirListingMetadata dirMeta = ms.listChildren(strToPath(parent));
if (!allowMissing() || dirMeta != null) {
+ dirMeta = dirMeta.withoutTombstones();
assertNotNull("list after putListStatus", dirMeta);
Collection<PathMetadata> entries = dirMeta.getListing();
assertNotNull("listStatus has entries", entries);
@@ -700,6 +761,7 @@ public abstract class MetadataStoreTestBase extends Assert {
assertNotNull("Directory " + pathStr + " in cache", dirMeta);
}
if (!allowMissing() || dirMeta != null) {
+ dirMeta = dirMeta.withoutTombstones();
assertEquals("Number of entries in dir " + pathStr, size,
nonDeleted(dirMeta.getListing()).size());
}
@@ -708,21 +770,27 @@ public abstract class MetadataStoreTestBase extends Assert {
/** @return only file statuses which are *not* marked deleted. */
private Collection<PathMetadata> nonDeleted(
Collection<PathMetadata> statuses) {
- /* TODO: filter out paths marked for deletion. */
- return statuses;
+ Collection<PathMetadata> currentStatuses = new ArrayList<>();
+ for (PathMetadata status : statuses) {
+ if (!status.isDeleted()) {
+ currentStatuses.add(status);
+ }
+ }
+ return currentStatuses;
}
- private void assertNotCached(String pathStr) throws IOException {
- // TODO this should return an entry with deleted flag set
+ private void assertDeleted(String pathStr) throws IOException {
Path path = strToPath(pathStr);
PathMetadata meta = ms.get(path);
- assertNull(pathStr + " should not be cached.", meta);
+ boolean cached = meta != null && !meta.isDeleted();
+ assertFalse(pathStr + " should not be cached.", cached);
}
protected void assertCached(String pathStr) throws IOException {
Path path = strToPath(pathStr);
PathMetadata meta = ms.get(path);
- assertNotNull(pathStr + " should be cached.", meta);
+ boolean cached = meta != null && !meta.isDeleted();
+ assertTrue(pathStr + " should be cached.", cached);
}
/**
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
index 3584b54..27416bb 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
@@ -294,7 +294,7 @@ public class TestDynamoDBMetadataStore extends MetadataStoreTestBase {
if (oldMetas != null) {
// put all metadata of old paths and verify
ms.put(new DirListingMetadata(oldDir, oldMetas, false));
- assertEquals(0, ms.listChildren(newDir).numEntries());
+ assertEquals(0, ms.listChildren(newDir).withoutTombstones().numEntries());
assertTrue(CollectionUtils.isEqualCollection(oldMetas,
ms.listChildren(oldDir).getListing()));
@@ -306,7 +306,7 @@ public class TestDynamoDBMetadataStore extends MetadataStoreTestBase {
// move the old paths to new paths and verify
ms.move(pathsToDelete, newMetas);
- assertEquals(0, ms.listChildren(oldDir).numEntries());
+ assertEquals(0, ms.listChildren(oldDir).withoutTombstones().numEntries());
if (newMetas != null) {
assertTrue(CollectionUtils.isEqualCollection(newMetas,
ms.listChildren(newDir).getListing()));
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
index 4cffc6f..89d0498 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
@@ -75,7 +75,7 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
@Test
public void testClearByAncestor() {
- Map<Path, String> map = new HashMap<>();
+ Map<Path, PathMetadata> map = new HashMap<>();
// 1. Test paths without scheme/host
assertClearResult(map, "", "/", 0);
@@ -90,21 +90,37 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
assertClearResult(map, p, "/invalid", 5);
}
- private static void populateMap(Map<Path, String> map, String prefix) {
- String dummyVal = "dummy";
- map.put(new Path(prefix + "/dirA/dirB/"), dummyVal);
- map.put(new Path(prefix + "/dirA/dirB/dirC"), dummyVal);
- map.put(new Path(prefix + "/dirA/dirB/dirC/file1"), dummyVal);
- map.put(new Path(prefix + "/dirA/dirB/dirC/file2"), dummyVal);
- map.put(new Path(prefix + "/dirA/file1"), dummyVal);
+ private static void populateMap(Map<Path, PathMetadata> map,
+ String prefix) {
+ populateEntry(map, new Path(prefix + "/dirA/dirB/"));
+ populateEntry(map, new Path(prefix + "/dirA/dirB/dirC"));
+ populateEntry(map, new Path(prefix + "/dirA/dirB/dirC/file1"));
+ populateEntry(map, new Path(prefix + "/dirA/dirB/dirC/file2"));
+ populateEntry(map, new Path(prefix + "/dirA/file1"));
}
- private static void assertClearResult(Map <Path, String> map,
+ private static void populateEntry(Map<Path, PathMetadata> map,
+ Path path) {
+ map.put(path, new PathMetadata(new FileStatus(0, true, 0, 0, 0, path)));
+ }
+
+ private static int sizeOfMap(Map<Path, PathMetadata> map) {
+ int count = 0;
+ for (PathMetadata meta : map.values()) {
+ if (!meta.isDeleted()) {
+ count++;
+ }
+ }
+ return count;
+ }
+
+ private static void assertClearResult(Map <Path, PathMetadata> map,
String prefixStr, String pathStr, int leftoverSize) {
populateMap(map, prefixStr);
- LocalMetadataStore.clearHashByAncestor(new Path(prefixStr + pathStr), map);
+ LocalMetadataStore.deleteHashByAncestor(new Path(prefixStr + pathStr), map,
+ true);
assertEquals(String.format("Map should have %d entries", leftoverSize),
- leftoverSize, map.size());
+ leftoverSize, sizeOfMap(map));
map.clear();
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/2f3305db/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
index 9b8e3c1..876cc80 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
@@ -114,7 +114,7 @@ public abstract class AbstractITestS3AMetadataStoreScale extends
describe("Running move workload");
NanoTimer moveTimer = new NanoTimer();
LOG.info("Running {} moves of {} paths each", operations,
- origPaths.size());
+ origMetas.size());
for (int i = 0; i < operations; i++) {
Collection<Path> toDelete;
Collection<PathMetadata> toCreate;
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[5/6] hadoop git commit: Revert "HADOOP-13760. S3Guard: add delete
tracking."
Posted by ma...@apache.org.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
deleted file mode 100644
index 43eb2c0..0000000
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestListing.java
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.hadoop.fs.s3a;
-
-import org.apache.hadoop.fs.FileStatus;
-import org.apache.hadoop.fs.LocatedFileStatus;
-import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.RemoteIterator;
-import org.junit.Assert;
-import org.junit.Test;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Set;
-
-/**
- * Place for the S3A listing classes; keeps all the small classes under control.
- */
-public class TestListing extends AbstractS3AMockTest {
-
- private static class MockRemoteIterator<FileStatus> implements
- RemoteIterator<FileStatus> {
- private Iterator<FileStatus> iterator;
-
- MockRemoteIterator(Collection<FileStatus> source) {
- iterator = source.iterator();
- }
-
- public boolean hasNext() {
- return iterator.hasNext();
- }
-
- public FileStatus next() {
- return iterator.next();
- }
- }
-
- private FileStatus blankFileStatus(Path path) {
- return new FileStatus(0, true, 0, 0, 0, path);
- }
-
- @Test
- public void testTombstoneReconcilingIterator() throws Exception {
- Path parent = new Path("/parent");
- Path liveChild = new Path(parent, "/liveChild");
- Path deletedChild = new Path(parent, "/deletedChild");
- Path[] allFiles = {parent, liveChild, deletedChild};
- Path[] liveFiles = {parent, liveChild};
-
- Listing listing = new Listing(fs);
- Collection<FileStatus> statuses = new ArrayList<>();
- statuses.add(blankFileStatus(parent));
- statuses.add(blankFileStatus(liveChild));
- statuses.add(blankFileStatus(deletedChild));
-
- Set<Path> tombstones = new HashSet<>();
- tombstones.add(deletedChild);
-
- RemoteIterator<FileStatus> sourceIterator = new MockRemoteIterator(
- statuses);
- RemoteIterator<LocatedFileStatus> locatedIterator =
- listing.createLocatedFileStatusIterator(sourceIterator);
- RemoteIterator<LocatedFileStatus> reconcilingIterator =
- listing.createTombstoneReconcilingIterator(locatedIterator, tombstones);
-
- Set<Path> expectedPaths = new HashSet<>();
- expectedPaths.add(parent);
- expectedPaths.add(liveChild);
-
- Set<Path> actualPaths = new HashSet<>();
- while (reconcilingIterator.hasNext()) {
- actualPaths.add(reconcilingIterator.next().getPath());
- }
- Assert.assertTrue(actualPaths.equals(expectedPaths));
- }
-}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/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 dfa8a9e..99acf6e 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
@@ -21,7 +21,6 @@ package org.apache.hadoop.fs.s3a.s3guard;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.fs.s3a.S3ATestUtils;
import org.apache.hadoop.fs.s3a.Tristate;
@@ -135,52 +134,12 @@ public abstract class MetadataStoreTestBase extends Assert {
}
/**
- * Helper function for verifying DescendantsIterator and
- * MetadataStoreListFilesIterator behavior.
- * @param createNodes List of paths to create
- * @param checkNodes List of paths that the iterator should return
- * @throws IOException
- */
- private void doTestDescendantsIterator(
- Class implementation, String[] createNodes,
- String[] checkNodes) throws Exception {
- // we set up the example file system tree in metadata store
- for (String pathStr : createNodes) {
- final FileStatus status = pathStr.contains("file")
- ? basicFileStatus(strToPath(pathStr), 100, false)
- : basicFileStatus(strToPath(pathStr), 0, true);
- ms.put(new PathMetadata(status));
- }
-
- final PathMetadata rootMeta = new PathMetadata(makeDirStatus("/"));
- RemoteIterator<FileStatus> iterator;
- if (implementation == DescendantsIterator.class) {
- iterator = new DescendantsIterator(ms, rootMeta);
- } else if (implementation == MetadataStoreListFilesIterator.class) {
- iterator = new MetadataStoreListFilesIterator(ms, rootMeta, false);
- } else {
- throw new UnsupportedOperationException("Unrecognized class");
- }
-
- final Set<String> actual = new HashSet<>();
- while (iterator.hasNext()) {
- final Path p = iterator.next().getPath();
- actual.add(Path.getPathWithoutSchemeAndAuthority(p).toString());
- }
- LOG.info("We got {} by iterating DescendantsIterator", actual);
-
- if (!allowMissing()) {
- assertEquals(Sets.newHashSet(checkNodes), actual);
- }
- }
-
- /**
* Test that we can get the whole sub-tree by iterating DescendantsIterator.
*
* The tree is similar to or same as the example in code comment.
*/
@Test
- public void testDescendantsIterator() throws Exception {
+ public void testDescendantsIterator() throws IOException {
final String[] tree = new String[] {
"/dir1",
"/dir1/dir2",
@@ -193,38 +152,26 @@ public abstract class MetadataStoreTestBase extends Assert {
"/dir1/dir3/dir5/file4",
"/dir1/dir3/dir6"
};
- doTestDescendantsIterator(DescendantsIterator.class,
- tree, tree);
- }
+ // we set up the example file system tree in metadata store
+ for (String pathStr : tree) {
+ final FileStatus status = pathStr.contains("file")
+ ? basicFileStatus(strToPath(pathStr), 100, false)
+ : basicFileStatus(strToPath(pathStr), 0, true);
+ ms.put(new PathMetadata(status));
+ }
- /**
- * Test that we can get the correct subset of the tree with
- * MetadataStoreListFilesIterator.
- *
- * The tree is similar to or same as the example in code comment.
- */
- @Test
- public void testMetadataStoreListFilesIterator() throws Exception {
- final String[] wholeTree = new String[] {
- "/dir1",
- "/dir1/dir2",
- "/dir1/dir3",
- "/dir1/dir2/file1",
- "/dir1/dir2/file2",
- "/dir1/dir3/dir4",
- "/dir1/dir3/dir5",
- "/dir1/dir3/dir4/file3",
- "/dir1/dir3/dir5/file4",
- "/dir1/dir3/dir6"
- };
- final String[] leafNodes = new String[] {
- "/dir1/dir2/file1",
- "/dir1/dir2/file2",
- "/dir1/dir3/dir4/file3",
- "/dir1/dir3/dir5/file4"
- };
- doTestDescendantsIterator(MetadataStoreListFilesIterator.class, wholeTree,
- leafNodes);
+ final Set<String> actual = new HashSet<>();
+ final PathMetadata rootMeta = new PathMetadata(makeDirStatus("/"));
+ for (DescendantsIterator desc = new DescendantsIterator(ms, rootMeta);
+ desc.hasNext();) {
+ final Path p = desc.next().getPath();
+ actual.add(Path.getPathWithoutSchemeAndAuthority(p).toString());
+ }
+ LOG.info("We got {} by iterating DescendantsIterator", actual);
+
+ if (!allowMissing()) {
+ assertEquals(Sets.newHashSet(tree), actual);
+ }
}
@Test
@@ -311,7 +258,7 @@ public abstract class MetadataStoreTestBase extends Assert {
/* Ensure delete happened. */
assertDirectorySize("/ADirectory1/db1", 1);
PathMetadata meta = ms.get(strToPath("/ADirectory1/db1/file2"));
- assertTrue("File deleted", meta == null || meta.isDeleted());
+ assertNull("File deleted", meta);
}
@Test
@@ -337,10 +284,10 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.deleteSubtree(strToPath(p + "/ADirectory1/db1/"));
assertEmptyDirectory(p + "/ADirectory1");
- assertDeleted(p + "/ADirectory1/db1");
- assertDeleted(p + "/ADirectory1/file1");
- assertDeleted(p + "/ADirectory1/file2");
- assertDeleted(p + "/ADirectory1/db1/dc1/dd1/deepFile");
+ assertNotCached(p + "/ADirectory1/db1");
+ assertNotCached(p + "/ADirectory1/file1");
+ assertNotCached(p + "/ADirectory1/file2");
+ assertNotCached(p + "/ADirectory1/db1/dc1/dd1/deepFile");
assertEmptyDirectory(p + "/ADirectory2");
}
@@ -355,11 +302,11 @@ public abstract class MetadataStoreTestBase extends Assert {
setUpDeleteTest();
ms.deleteSubtree(strToPath("/"));
- assertDeleted("/ADirectory1");
- assertDeleted("/ADirectory2");
- assertDeleted("/ADirectory2/db1");
- assertDeleted("/ADirectory2/db1/file1");
- assertDeleted("/ADirectory2/db1/file2");
+ assertNotCached("/ADirectory1");
+ assertNotCached("/ADirectory2");
+ assertNotCached("/ADirectory2/db1");
+ assertNotCached("/ADirectory2/db1/file1");
+ assertNotCached("/ADirectory2/db1/file2");
}
@Test
@@ -403,12 +350,6 @@ public abstract class MetadataStoreTestBase extends Assert {
verifyFileStatus(meta.getFileStatus(), 100);
}
- if (!(ms instanceof NullMetadataStore)) {
- ms.delete(strToPath(filePath));
- meta = ms.get(strToPath(filePath));
- assertTrue("Tombstone not left for deleted file", meta.isDeleted());
- }
-
meta = ms.get(strToPath(dirPath));
if (!allowMissing() || meta != null) {
assertNotNull("Get found file (dir)", meta);
@@ -500,7 +441,6 @@ public abstract class MetadataStoreTestBase extends Assert {
dirMeta = ms.listChildren(strToPath("/a1"));
if (!allowMissing() || dirMeta != null) {
- dirMeta = dirMeta.withoutTombstones();
assertListingsEqual(dirMeta.getListing(), "/a1/b1", "/a1/b2");
}
@@ -546,7 +486,6 @@ public abstract class MetadataStoreTestBase extends Assert {
Collection<PathMetadata> entries;
DirListingMetadata dirMeta = ms.listChildren(strToPath("/"));
if (!allowMissing() || dirMeta != null) {
- dirMeta = dirMeta.withoutTombstones();
assertNotNull("Listing root", dirMeta);
entries = dirMeta.getListing();
assertListingsEqual(entries, "/a1", "/a2", "/a3");
@@ -574,12 +513,13 @@ public abstract class MetadataStoreTestBase extends Assert {
dirMeta = ms.listChildren(strToPath("/a1"));
if (!allowMissing() || dirMeta != null) {
assertNotNull("Listing /a1", dirMeta);
- entries = dirMeta.withoutTombstones().getListing();
+ entries = dirMeta.getListing();
assertListingsEqual(entries, "/a1/b2");
}
PathMetadata meta = ms.get(strToPath("/a1/b1/file1"));
- assertTrue("Src path deleted", meta == null || meta.isDeleted());
+ // TODO allow return of PathMetadata with isDeleted == true
+ assertNull("Src path deleted", meta);
// Assert dest looks right
meta = ms.get(strToPath("/b1/file1"));
@@ -656,7 +596,7 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.prune(cutoff);
ls = ms.listChildren(strToPath("/pruneFiles"));
if (allowMissing()) {
- assertDeleted("/pruneFiles/old");
+ assertNotCached("/pruneFiles/old");
} else {
assertListingsEqual(ls.getListing(), "/pruneFiles/new");
}
@@ -685,7 +625,7 @@ public abstract class MetadataStoreTestBase extends Assert {
ms.prune(cutoff);
- assertDeleted("/pruneDirs/dir/file");
+ assertNotCached("/pruneDirs/dir/file");
}
/*
@@ -706,7 +646,6 @@ public abstract class MetadataStoreTestBase extends Assert {
"file3"));
DirListingMetadata dirMeta = ms.listChildren(strToPath(parent));
if (!allowMissing() || dirMeta != null) {
- dirMeta = dirMeta.withoutTombstones();
assertNotNull("list after putListStatus", dirMeta);
Collection<PathMetadata> entries = dirMeta.getListing();
assertNotNull("listStatus has entries", entries);
@@ -761,7 +700,6 @@ public abstract class MetadataStoreTestBase extends Assert {
assertNotNull("Directory " + pathStr + " in cache", dirMeta);
}
if (!allowMissing() || dirMeta != null) {
- dirMeta = dirMeta.withoutTombstones();
assertEquals("Number of entries in dir " + pathStr, size,
nonDeleted(dirMeta.getListing()).size());
}
@@ -770,27 +708,21 @@ public abstract class MetadataStoreTestBase extends Assert {
/** @return only file statuses which are *not* marked deleted. */
private Collection<PathMetadata> nonDeleted(
Collection<PathMetadata> statuses) {
- Collection<PathMetadata> currentStatuses = new ArrayList<>();
- for (PathMetadata status : statuses) {
- if (!status.isDeleted()) {
- currentStatuses.add(status);
- }
- }
- return currentStatuses;
+ /* TODO: filter out paths marked for deletion. */
+ return statuses;
}
- private void assertDeleted(String pathStr) throws IOException {
+ private void assertNotCached(String pathStr) throws IOException {
+ // TODO this should return an entry with deleted flag set
Path path = strToPath(pathStr);
PathMetadata meta = ms.get(path);
- boolean cached = meta != null && !meta.isDeleted();
- assertFalse(pathStr + " should not be cached.", cached);
+ assertNull(pathStr + " should not be cached.", meta);
}
protected void assertCached(String pathStr) throws IOException {
Path path = strToPath(pathStr);
PathMetadata meta = ms.get(path);
- boolean cached = meta != null && !meta.isDeleted();
- assertTrue(pathStr + " should be cached.", cached);
+ assertNotNull(pathStr + " should be cached.", meta);
}
/**
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
index 27416bb..3584b54 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMetadataStore.java
@@ -294,7 +294,7 @@ public class TestDynamoDBMetadataStore extends MetadataStoreTestBase {
if (oldMetas != null) {
// put all metadata of old paths and verify
ms.put(new DirListingMetadata(oldDir, oldMetas, false));
- assertEquals(0, ms.listChildren(newDir).withoutTombstones().numEntries());
+ assertEquals(0, ms.listChildren(newDir).numEntries());
assertTrue(CollectionUtils.isEqualCollection(oldMetas,
ms.listChildren(oldDir).getListing()));
@@ -306,7 +306,7 @@ public class TestDynamoDBMetadataStore extends MetadataStoreTestBase {
// move the old paths to new paths and verify
ms.move(pathsToDelete, newMetas);
- assertEquals(0, ms.listChildren(oldDir).withoutTombstones().numEntries());
+ assertEquals(0, ms.listChildren(oldDir).numEntries());
if (newMetas != null) {
assertTrue(CollectionUtils.isEqualCollection(newMetas,
ms.listChildren(newDir).getListing()));
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
index 89d0498..4cffc6f 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
@@ -75,7 +75,7 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
@Test
public void testClearByAncestor() {
- Map<Path, PathMetadata> map = new HashMap<>();
+ Map<Path, String> map = new HashMap<>();
// 1. Test paths without scheme/host
assertClearResult(map, "", "/", 0);
@@ -90,37 +90,21 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
assertClearResult(map, p, "/invalid", 5);
}
- private static void populateMap(Map<Path, PathMetadata> map,
- String prefix) {
- populateEntry(map, new Path(prefix + "/dirA/dirB/"));
- populateEntry(map, new Path(prefix + "/dirA/dirB/dirC"));
- populateEntry(map, new Path(prefix + "/dirA/dirB/dirC/file1"));
- populateEntry(map, new Path(prefix + "/dirA/dirB/dirC/file2"));
- populateEntry(map, new Path(prefix + "/dirA/file1"));
+ private static void populateMap(Map<Path, String> map, String prefix) {
+ String dummyVal = "dummy";
+ map.put(new Path(prefix + "/dirA/dirB/"), dummyVal);
+ map.put(new Path(prefix + "/dirA/dirB/dirC"), dummyVal);
+ map.put(new Path(prefix + "/dirA/dirB/dirC/file1"), dummyVal);
+ map.put(new Path(prefix + "/dirA/dirB/dirC/file2"), dummyVal);
+ map.put(new Path(prefix + "/dirA/file1"), dummyVal);
}
- private static void populateEntry(Map<Path, PathMetadata> map,
- Path path) {
- map.put(path, new PathMetadata(new FileStatus(0, true, 0, 0, 0, path)));
- }
-
- private static int sizeOfMap(Map<Path, PathMetadata> map) {
- int count = 0;
- for (PathMetadata meta : map.values()) {
- if (!meta.isDeleted()) {
- count++;
- }
- }
- return count;
- }
-
- private static void assertClearResult(Map <Path, PathMetadata> map,
+ private static void assertClearResult(Map <Path, String> map,
String prefixStr, String pathStr, int leftoverSize) {
populateMap(map, prefixStr);
- LocalMetadataStore.deleteHashByAncestor(new Path(prefixStr + pathStr), map,
- true);
+ LocalMetadataStore.clearHashByAncestor(new Path(prefixStr + pathStr), map);
assertEquals(String.format("Map should have %d entries", leftoverSize),
- leftoverSize, sizeOfMap(map));
+ leftoverSize, map.size());
map.clear();
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
index 876cc80..9b8e3c1 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractITestS3AMetadataStoreScale.java
@@ -114,7 +114,7 @@ public abstract class AbstractITestS3AMetadataStoreScale extends
describe("Running move workload");
NanoTimer moveTimer = new NanoTimer();
LOG.info("Running {} moves of {} paths each", operations,
- origMetas.size());
+ origPaths.size());
for (int i = 0; i < operations; i++) {
Collection<Path> toDelete;
Collection<PathMetadata> toCreate;
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[2/6] hadoop git commit: Reproduced erroneous authoritative listing
Posted by ma...@apache.org.
Reproduced erroneous authoritative listing
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/83b82348
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/83b82348
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/83b82348
Branch: refs/heads/HADOOP-13345
Commit: 83b82348ff8a4bd347b5c868ebdd87df3961a5a0
Parents: 2f3305d
Author: Sean Mackrory <ma...@apache.org>
Authored: Wed May 24 09:19:01 2017 -0600
Committer: Sean Mackrory <ma...@apache.org>
Committed: Wed May 24 17:19:39 2017 -0600
----------------------------------------------------------------------
.../org/apache/hadoop/fs/s3a/S3AFileSystem.java | 2 +-
.../fs/contract/s3a/ITestS3AContractRename.java | 47 ++++++++++++++++++++
.../fs/s3a/s3guard/TestLocalMetadataStore.java | 2 +-
3 files changed, 49 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/83b82348/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
index 7de94cc..ffb2147 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
@@ -960,7 +960,7 @@ public class S3AFileSystem extends FileSystem {
}
@VisibleForTesting
- MetadataStore getMetadataStore() {
+ public MetadataStore getMetadataStore() {
return metadataStore;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/83b82348/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
index 4339649..65da13a 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
@@ -23,6 +23,14 @@ import org.apache.hadoop.fs.contract.AbstractContractRenameTest;
import org.apache.hadoop.fs.contract.AbstractFSContract;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.s3a.S3AFileSystem;
+import org.apache.hadoop.fs.s3a.Tristate;
+import org.apache.hadoop.fs.s3a.s3guard.DirListingMetadata;
+import org.apache.hadoop.fs.s3a.s3guard.MetadataStore;
+import org.apache.hadoop.fs.s3a.s3guard.S3Guard;
+import org.junit.Test;
+
+import java.io.IOException;
import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
@@ -72,4 +80,43 @@ public class ITestS3AContractRename extends AbstractContractRenameTest {
boolean rename = fs.rename(srcDir, destDir);
assertFalse("s3a doesn't support rename to non-empty directory", rename);
}
+
+ @Override
+ @Test
+ public void testRenamePopulatesFileAncestors() throws IOException {
+ final FileSystem fs = getFileSystem();
+ final Path src = path("testRenamePopulatesFileAncestors/source");
+ fs.mkdirs(src);
+ final String nestedFile = "/dir1/dir2/dir3/file4";
+ byte[] srcDataset = dataset(256, 'a', 'z');
+ writeDataset(fs, path(src + nestedFile), srcDataset, srcDataset.length,
+ 1024, false);
+
+ Path dst = path("testRenamePopulatesFileAncestorsNew");
+
+ DirListingMetadata l = ((S3AFileSystem)fs).getMetadataStore()
+ .listChildren(src);
+ fs.rename(src, dst);
+ }
+
+ @Test
+ public void testMkdirPopulatesFileAncestors() throws IOException {
+ final FileSystem fs = getFileSystem();
+ final MetadataStore ms = ((S3AFileSystem) fs).getMetadataStore();
+ final Path parent = path("testMkdirPopulatesFileAncestors/source");
+ try {
+ //fs.mkdirs(parent);
+ final Path nestedFile = new Path(parent, "/dir1/dir2/dir3/file4");
+ byte[] srcDataset = dataset(256, 'a', 'z');
+ writeDataset(fs, nestedFile, srcDataset, srcDataset.length,
+ 1024, false);
+
+ DirListingMetadata list = ms.listChildren(parent);
+ System.out.println("list isEmpty: " + list.isEmpty());
+ System.out.println("list isAuthoritative: " + list.isAuthoritative());
+ assertTrue(list.isEmpty() == Tristate.FALSE || !list.isAuthoritative());
+ } finally {
+ fs.delete(parent, true);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/83b82348/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
index 89d0498..81183e70 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestLocalMetadataStore.java
@@ -24,6 +24,7 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.S3ATestUtils;
+import org.apache.hadoop.fs.s3a.Tristate;
import org.junit.Test;
import java.io.IOException;
@@ -136,5 +137,4 @@ public class TestLocalMetadataStore extends MetadataStoreTestBase {
S3ATestUtils.verifyDirStatus(status, REPLICATION, getModTime(),
getAccessTime(), OWNER, GROUP, PERMISSION);
}
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[6/6] hadoop git commit: Revert "HADOOP-13760. S3Guard: add delete
tracking."
Posted by ma...@apache.org.
Revert "HADOOP-13760. S3Guard: add delete tracking."
This reverts commit 2f3305db768e4159b9216de88adbd73b686d9a02.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c1775960
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c1775960
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c1775960
Branch: refs/heads/HADOOP-13345
Commit: c17759607e7ce770aa4635a4896ac1233b003f33
Parents: 447d7b8
Author: Sean Mackrory <ma...@apache.org>
Authored: Wed May 24 20:06:28 2017 -0600
Committer: Sean Mackrory <ma...@apache.org>
Committed: Wed May 24 20:06:28 2017 -0600
----------------------------------------------------------------------
.../fs/s3a/InconsistentAmazonS3Client.java | 122 +-------
.../java/org/apache/hadoop/fs/s3a/Listing.java | 91 ------
.../org/apache/hadoop/fs/s3a/S3AFileSystem.java | 310 ++++++-------------
.../fs/s3a/s3guard/DescendantsIterator.java | 14 +-
.../fs/s3a/s3guard/DirListingMetadata.java | 32 --
.../fs/s3a/s3guard/DynamoDBMetadataStore.java | 86 ++---
.../fs/s3a/s3guard/LocalMetadataStore.java | 61 ++--
.../hadoop/fs/s3a/s3guard/MetadataStore.java | 19 +-
.../s3guard/MetadataStoreListFilesIterator.java | 168 ----------
.../fs/s3a/s3guard/NullMetadataStore.java | 5 -
.../hadoop/fs/s3a/s3guard/PathMetadata.java | 27 +-
.../PathMetadataDynamoDBTranslation.java | 7 +-
.../apache/hadoop/fs/s3a/s3guard/S3Guard.java | 26 +-
.../hadoop/fs/s3a/s3guard/S3GuardTool.java | 5 +-
.../hadoop/fs/s3a/ITestS3GuardEmptyDirs.java | 62 ++--
.../fs/s3a/ITestS3GuardListConsistency.java | 213 +------------
.../org/apache/hadoop/fs/s3a/TestListing.java | 94 ------
.../fs/s3a/s3guard/MetadataStoreTestBase.java | 150 +++------
.../s3a/s3guard/TestDynamoDBMetadataStore.java | 4 +-
.../fs/s3a/s3guard/TestLocalMetadataStore.java | 38 +--
.../AbstractITestS3AMetadataStoreScale.java | 2 +-
21 files changed, 276 insertions(+), 1260 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
index aadcc37..ebca268 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/InconsistentAmazonS3Client.java
@@ -23,7 +23,6 @@ import com.amazonaws.AmazonServiceException;
import com.amazonaws.ClientConfiguration;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.services.s3.AmazonS3Client;
-import com.amazonaws.services.s3.model.DeleteObjectRequest;
import com.amazonaws.services.s3.model.ListObjectsRequest;
import com.amazonaws.services.s3.model.ObjectListing;
import com.amazonaws.services.s3.model.PutObjectRequest;
@@ -34,7 +33,6 @@ import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -59,50 +57,14 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
private static final Logger LOG =
LoggerFactory.getLogger(InconsistentAmazonS3Client.class);
- /**
- * Composite of data we need to track about recently deleted objects:
- * when it was deleted (same was with recently put objects) and the object
- * summary (since we should keep returning it for sometime after its
- * deletion).
- */
- private static class Delete {
- private Long time;
- private S3ObjectSummary summary;
-
- Delete(Long time, S3ObjectSummary summary) {
- this.time = time;
- this.summary = summary;
- }
-
- public Long time() {
- return time;
- }
-
- public S3ObjectSummary summary() {
- return summary;
- }
- }
-
- /** Map of key to delay -> time it was deleted + object summary (object
- * summary is null for prefixes. */
- private Map<String, Delete> delayedDeletes = new HashMap<>();
-
/** Map of key to delay -> time it was created. */
- private Map<String, Long> delayedPutKeys = new HashMap<>();
+ private Map<String, Long> delayedKeys = new HashMap<>();
public InconsistentAmazonS3Client(AWSCredentialsProvider credentials,
ClientConfiguration clientConfiguration) {
super(credentials, clientConfiguration);
}
- @Override
- public void deleteObject(DeleteObjectRequest deleteObjectRequest)
- throws AmazonClientException, AmazonServiceException {
- LOG.debug("key {}", deleteObjectRequest.getKey());
- registerDeleteObject(deleteObjectRequest);
- super.deleteObject(deleteObjectRequest);
- }
-
/* We should only need to override this version of putObject() */
@Override
public PutObjectResult putObject(PutObjectRequest putObjectRequest)
@@ -118,46 +80,10 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
throws AmazonClientException, AmazonServiceException {
LOG.debug("prefix {}", listObjectsRequest.getPrefix());
ObjectListing listing = super.listObjects(listObjectsRequest);
- listing = filterListObjects(listObjectsRequest, listing);
- listing = restoreListObjects(listObjectsRequest, listing);
- return listing;
+ return filterListObjects(listObjectsRequest,
+ listing);
}
- private boolean addIfNotPresent(List<S3ObjectSummary> list,
- S3ObjectSummary item) {
- // Behavior of S3ObjectSummary
- String key = item.getKey();
- for (S3ObjectSummary member : list) {
- if (member.getKey().equals(key)) {
- return false;
- }
- }
- return list.add(item);
- }
-
- private ObjectListing restoreListObjects(ListObjectsRequest request,
- ObjectListing rawListing) {
- List<S3ObjectSummary> outputList = rawListing.getObjectSummaries();
- List<String> outputPrefixes = rawListing.getCommonPrefixes();
- for (String key : new HashSet<>(delayedDeletes.keySet())) {
- Delete delete = delayedDeletes.get(key);
- if (isKeyDelayed(delete.time(), key)) {
- if (key.startsWith(request.getPrefix())) {
- if (delete.summary == null) {
- if (!outputPrefixes.contains(key)) {
- outputPrefixes.add(key);
- }
- } else {
- addIfNotPresent(outputList, delete.summary());
- }
- }
- } else {
- delayedDeletes.remove(key);
- }
- }
-
- return new CustomObjectListing(rawListing, outputList, outputPrefixes);
- }
private ObjectListing filterListObjects(ListObjectsRequest request,
ObjectListing rawListing) {
@@ -165,8 +91,7 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
// Filter object listing
List<S3ObjectSummary> outputList = new ArrayList<>();
for (S3ObjectSummary s : rawListing.getObjectSummaries()) {
- String key = s.getKey();
- if (!isKeyDelayed(delayedPutKeys.get(key), key)) {
+ if (!isVisibilityDelayed(s.getKey())) {
outputList.add(s);
}
}
@@ -174,7 +99,7 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
// Filter prefixes (directories)
List<String> outputPrefixes = new ArrayList<>();
for (String key : rawListing.getCommonPrefixes()) {
- if (!isKeyDelayed(delayedPutKeys.get(key), key)) {
+ if (!isVisibilityDelayed(key)) {
outputPrefixes.add(key);
}
}
@@ -182,43 +107,28 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
return new CustomObjectListing(rawListing, outputList, outputPrefixes);
}
- private boolean isKeyDelayed(Long enqueueTime, String key) {
- if (enqueueTime == null) {
+ private boolean isVisibilityDelayed(String key) {
+ Long createTime = delayedKeys.get(key);
+ if (createTime == null) {
LOG.debug("no delay for key {}", key);
return false;
}
long currentTime = System.currentTimeMillis();
- long deadline = enqueueTime + DELAY_KEY_MILLIS;
+ long deadline = createTime + DELAY_KEY_MILLIS;
if (currentTime >= deadline) {
- delayedDeletes.remove(key);
- LOG.debug("no longer delaying {}", key);
+ delayedKeys.remove(key);
+ LOG.debug("{} no longer delayed", key);
return false;
} else {
- LOG.info("delaying {}", key);
+ LOG.info("{} delaying visibility", key);
return true;
}
}
- private void registerDeleteObject(DeleteObjectRequest req) {
- String key = req.getKey();
- if (shouldDelay(key)) {
- // Record summary so we can add it back for some time post-deletion
- S3ObjectSummary summary = null;
- ObjectListing list = listObjects(req.getBucketName(), key);
- for (S3ObjectSummary result : list.getObjectSummaries()) {
- if (result.getKey().equals(key)) {
- summary = result;
- break;
- }
- }
- delayedDeletes.put(key, new Delete(System.currentTimeMillis(), summary));
- }
- }
-
private void registerPutObject(PutObjectRequest req) {
String key = req.getKey();
if (shouldDelay(key)) {
- enqueueDelayedPut(key);
+ enqueueDelayKey(key);
}
}
@@ -238,9 +148,9 @@ public class InconsistentAmazonS3Client extends AmazonS3Client {
* listObject replies for a while, to simulate eventual list consistency.
* @param key key to delay visibility of
*/
- private void enqueueDelayedPut(String key) {
- LOG.debug("delaying put of {}", key);
- delayedPutKeys.put(key, System.currentTimeMillis());
+ private void enqueueDelayKey(String key) {
+ LOG.debug("key {}", key);
+ delayedKeys.put(key, System.currentTimeMillis());
}
/** Since ObjectListing is immutable, we just override it with wrapper. */
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
index 5da99b8..e91f2ec 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
@@ -22,7 +22,6 @@ import com.amazonaws.AmazonClientException;
import com.amazonaws.services.s3.model.ListObjectsRequest;
import com.amazonaws.services.s3.model.ObjectListing;
import com.amazonaws.services.s3.model.S3ObjectSummary;
-import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.LocatedFileStatus;
import org.apache.hadoop.fs.Path;
@@ -34,7 +33,6 @@ import org.slf4j.Logger;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -127,27 +125,12 @@ public class Listing {
* @param statusIterator an iterator over the remote status entries
* @return a new remote iterator
*/
- @VisibleForTesting
LocatedFileStatusIterator createLocatedFileStatusIterator(
RemoteIterator<FileStatus> statusIterator) {
return new LocatedFileStatusIterator(statusIterator);
}
/**
- * Create an located status iterator that wraps another to filter out a set
- * of recently deleted items.
- * @param iterator an iterator over the remote located status entries.
- * @param tombstones set of paths that are recently deleted and should be
- * filtered.
- * @return a new remote iterator.
- */
- @VisibleForTesting
- TombstoneReconcilingIterator createTombstoneReconcilingIterator(
- RemoteIterator<LocatedFileStatus> iterator, Set<Path> tombstones) {
- return new TombstoneReconcilingIterator(iterator, tombstones);
- }
-
- /**
* Interface to implement by the logic deciding whether to accept a summary
* entry or path as a valid file or directory.
*/
@@ -685,80 +668,6 @@ public class Listing {
}
/**
- * Wraps another iterator and filters out files that appear in the provided
- * set of tombstones. Will read ahead in the iterator when necessary to
- * ensure that emptiness is detected early enough if only deleted objects
- * remain in the source iterator.
- */
- static class TombstoneReconcilingIterator implements
- RemoteIterator<LocatedFileStatus> {
- private LocatedFileStatus next = null;
- private final RemoteIterator<LocatedFileStatus> iterator;
- private final Set<Path> tombstones;
-
- /**
- * @param iterator Source iterator to filter
- * @param tombstones set of tombstone markers to filter out of results
- */
- public TombstoneReconcilingIterator(RemoteIterator<LocatedFileStatus>
- iterator, Set<Path> tombstones) {
- this.iterator = iterator;
- if (tombstones != null) {
- this.tombstones = tombstones;
- } else {
- this.tombstones = Collections.EMPTY_SET;
- }
- }
-
- private boolean fetch() throws IOException {
- while (next == null && iterator.hasNext()) {
- LocatedFileStatus candidate = iterator.next();
- if (!tombstones.contains(candidate.getPath())) {
- next = candidate;
- return true;
- }
- }
- return false;
- }
-
- public boolean hasNext() throws IOException {
- if (next != null) {
- return true;
- }
- return fetch();
- }
-
- public LocatedFileStatus next() throws IOException {
- if (hasNext()) {
- LocatedFileStatus result = next;
- next = null;
- fetch();
- return result;
- }
- throw new NoSuchElementException();
- }
- }
-
- /**
- * Accept all entries except those which map to S3N pseudo directory markers.
- */
- static class AcceptAllButS3nDirs implements FileStatusAcceptor {
-
- public boolean accept(Path keyPath, S3ObjectSummary summary) {
- return !summary.getKey().endsWith(S3N_FOLDER_SUFFIX);
- }
-
- public boolean accept(Path keyPath, String prefix) {
- return !keyPath.toString().endsWith(S3N_FOLDER_SUFFIX);
- }
-
- public boolean accept(FileStatus status) {
- return !status.getPath().toString().endsWith(S3N_FOLDER_SUFFIX);
- }
-
- }
-
- /**
* Accept all entries except the base path and those which map to S3N
* pseudo directory markers.
*/
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
index 7de94cc..78b3970 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
@@ -26,13 +26,11 @@ import java.io.InterruptedIOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.Date;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import java.util.Objects;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.LinkedBlockingQueue;
@@ -95,8 +93,8 @@ import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.StorageStatistics;
import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.fs.s3a.s3guard.DescendantsIterator;
import org.apache.hadoop.fs.s3a.s3guard.DirListingMetadata;
-import org.apache.hadoop.fs.s3a.s3guard.MetadataStoreListFilesIterator;
import org.apache.hadoop.fs.s3a.s3guard.MetadataStore;
import org.apache.hadoop.fs.s3a.s3guard.PathMetadata;
import org.apache.hadoop.fs.s3a.s3guard.S3Guard;
@@ -880,44 +878,51 @@ public class S3AFileSystem extends FileSystem {
keysToDelete.add(new DeleteObjectsRequest.KeyVersion(dstKey));
}
- Path parentPath = keyToPath(srcKey);
- RemoteIterator<LocatedFileStatus> iterator = listFilesAndEmptyDirectories(
- parentPath, true);
- while (iterator.hasNext()) {
- LocatedFileStatus status = iterator.next();
- long length = status.getLen();
- String key = pathToKey(status.getPath());
- if (status.isDirectory() && !key.endsWith("/")) {
- key += "/";
- }
- keysToDelete
- .add(new DeleteObjectsRequest.KeyVersion(key));
- String newDstKey =
- dstKey + key.substring(srcKey.length());
- copyFile(key, newDstKey, length);
-
- if (hasMetadataStore()) {
- Path childSrc = keyToQualifiedPath(key);
- Path childDst = keyToQualifiedPath(newDstKey);
- if (objectRepresentsDirectory(key, length)) {
- S3Guard.addMoveDir(metadataStore, srcPaths, dstMetas, childSrc,
- childDst, username);
- } else {
- S3Guard.addMoveFile(metadataStore, srcPaths, dstMetas, childSrc,
- childDst, length, getDefaultBlockSize(childDst), username);
+ ListObjectsRequest request = new ListObjectsRequest();
+ request.setBucketName(bucket);
+ request.setPrefix(srcKey);
+ request.setMaxKeys(maxKeys);
+
+ ObjectListing objects = listObjects(request);
+
+ while (true) {
+ for (S3ObjectSummary summary : objects.getObjectSummaries()) {
+ long length = summary.getSize();
+ keysToDelete
+ .add(new DeleteObjectsRequest.KeyVersion(summary.getKey()));
+ String newDstKey =
+ dstKey + summary.getKey().substring(srcKey.length());
+ copyFile(summary.getKey(), newDstKey, length);
+
+ if (hasMetadataStore()) {
+ Path childSrc = keyToQualifiedPath(summary.getKey());
+ Path childDst = keyToQualifiedPath(newDstKey);
+ if (objectRepresentsDirectory(summary.getKey(), length)) {
+ S3Guard.addMoveDir(metadataStore, srcPaths, dstMetas, childSrc,
+ childDst, username);
+ } else {
+ S3Guard.addMoveFile(metadataStore, srcPaths, dstMetas, childSrc,
+ childDst, length, getDefaultBlockSize(childDst), username);
+ }
+ // Ancestor directories may not be listed, so we explicitly add them
+ S3Guard.addMoveAncestors(metadataStore, srcPaths, dstMetas,
+ keyToQualifiedPath(srcKey), childSrc, childDst, username);
+ }
+
+ if (keysToDelete.size() == MAX_ENTRIES_TO_DELETE) {
+ removeKeys(keysToDelete, true, false);
}
- // Ancestor directories may not be listed, so we explicitly add them
- S3Guard.addMoveAncestors(metadataStore, srcPaths, dstMetas,
- keyToQualifiedPath(srcKey), childSrc, childDst, username);
}
- if (keysToDelete.size() == MAX_ENTRIES_TO_DELETE) {
- removeKeys(keysToDelete, true, false);
+ if (objects.isTruncated()) {
+ objects = continueListObjects(objects);
+ } else {
+ if (!keysToDelete.isEmpty()) {
+ removeKeys(keysToDelete, false, false);
+ }
+ break;
}
}
- if (!keysToDelete.isEmpty()) {
- removeKeys(keysToDelete, false, false);
- }
// We moved all the children, now move the top-level dir
// Empty directory should have been added as the object summary
@@ -1627,42 +1632,6 @@ public class S3AFileSystem extends FileSystem {
throw translateException("innerMkdirs", path, e);
}
}
-
- private enum DirectoryStatus {
- DOES_NOT_EXIST, EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY,
- EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE, EXISTS_AND_IS_FILE
- }
-
- private DirectoryStatus checkPathForDirectory(Path path) throws
- IOException {
- try {
- if (path.isRoot()) {
- return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE;
- }
- String key = pathToKey(path);
-
- // Check MetadataStore, if any.
- FileStatus status = null;
- PathMetadata pm = metadataStore.get(path, false);
- if (pm != null) {
- if (pm.isDeleted()) {
- return DirectoryStatus.DOES_NOT_EXIST;
- }
- status = pm.getFileStatus();
- if (status != null && status.isDirectory()) {
- return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE;
- }
- }
- status = s3GetFileStatus(path, key, null);
- if (status.isDirectory()) {
- return DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY;
- }
- } catch (FileNotFoundException e) {
- return DirectoryStatus.DOES_NOT_EXIST;
- }
- return DirectoryStatus.EXISTS_AND_IS_FILE;
- }
-
/**
*
* Make the given path and all non-existent parents into
@@ -1670,70 +1639,64 @@ public class S3AFileSystem extends FileSystem {
* See {@link #mkdirs(Path, FsPermission)}
* @param p path to create
* @param permission to apply to f
- * @return true if a directory was created or already existed
+ * @return true if a directory was created
* @throws FileAlreadyExistsException there is a file at the path specified
* @throws IOException other IO problems
* @throws AmazonClientException on failures inside the AWS SDK
*/
private boolean innerMkdirs(Path p, FsPermission permission)
throws IOException, FileAlreadyExistsException, AmazonClientException {
- boolean createOnS3 = false;
Path f = qualify(p);
LOG.debug("Making directory: {}", f);
incrementStatistic(INVOCATION_MKDIRS);
+ FileStatus fileStatus;
List<Path> metadataStoreDirs = null;
if (hasMetadataStore()) {
metadataStoreDirs = new ArrayList<>();
}
- DirectoryStatus status = checkPathForDirectory(f);
- if (status == DirectoryStatus.DOES_NOT_EXIST) {
- createOnS3 = true;
- if (metadataStoreDirs != null) {
- metadataStoreDirs.add(f);
+ try {
+ fileStatus = getFileStatus(f);
+
+ if (fileStatus.isDirectory()) {
+ return true;
+ } else {
+ throw new FileAlreadyExistsException("Path is a file: " + f);
}
- } else if (status == DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY) {
+ } catch (FileNotFoundException e) {
+ // Walk path to root, ensuring closest ancestor is a directory, not file
+ Path fPart = f.getParent();
if (metadataStoreDirs != null) {
metadataStoreDirs.add(f);
}
- } else if (status == DirectoryStatus
- .EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE) {
- return true;
- } else if (status == DirectoryStatus.EXISTS_AND_IS_FILE) {
- throw new FileAlreadyExistsException("Path is a file: " + f);
- }
-
- // Walk path to root, ensuring closest ancestor is a directory, not file
- Path fPart = f.getParent();
- do {
- status = checkPathForDirectory(fPart);
- // The fake directory on S3 may not be visible immediately, but
- // only the leaf node has a fake directory on S3 so we can treat both
- // cases the same and just create a metadata store entry.
- if (status == DirectoryStatus.DOES_NOT_EXIST || status ==
- DirectoryStatus.EXISTS_AND_IS_DIRECTORY_ON_S3_ONLY) {
- if (metadataStoreDirs != null) {
- metadataStoreDirs.add(fPart);
+ do {
+ try {
+ fileStatus = getFileStatus(fPart);
+ if (fileStatus.isDirectory()) {
+ break;
+ }
+ if (fileStatus.isFile()) {
+ throw new FileAlreadyExistsException(String.format(
+ "Can't make directory for path '%s' since it is a file.",
+ fPart));
+ }
+ } catch (FileNotFoundException fnfe) {
+ instrumentation.errorIgnored();
+ // We create all missing directories in MetadataStore; it does not
+ // infer directories exist by prefix like S3.
+ if (metadataStoreDirs != null) {
+ metadataStoreDirs.add(fPart);
+ }
}
- } else if (status == DirectoryStatus
- .EXISTS_AND_IS_DIRECTORY_ON_METADATASTORE) {
- // do nothing - just break out of the loop, make whatever child
- // directories are needed on the metadata store, and return the
- // result.
- break;
- } else if (status == DirectoryStatus.EXISTS_AND_IS_FILE) {
- throw new FileAlreadyExistsException("Path is a file: " + f);
- }
- fPart = fPart.getParent();
- } while (fPart != null);
+ fPart = fPart.getParent();
+ } while (fPart != null);
- if (createOnS3) {
String key = pathToKey(f);
createFakeDirectory(key);
+ S3Guard.makeDirsOrdered(metadataStore, metadataStoreDirs, username);
+ deleteUnnecessaryFakeDirectories(f.getParent());
+ return true;
}
- S3Guard.makeDirsOrdered(metadataStore, metadataStoreDirs, username);
- deleteUnnecessaryFakeDirectories(f.getParent());
- return true;
}
/**
@@ -1766,12 +1729,8 @@ public class S3AFileSystem extends FileSystem {
// Check MetadataStore, if any.
PathMetadata pm = metadataStore.get(path, needEmptyDirectoryFlag);
- Set<Path> tombstones = Collections.EMPTY_SET;
if (pm != null) {
- if (pm.isDeleted()) {
- throw new FileNotFoundException("Path " + f + " is recorded as " +
- "deleted by S3Guard");
- }
+ // HADOOP-13760: handle deleted files, i.e. PathMetadata#isDeleted() here
FileStatus msStatus = pm.getFileStatus();
if (needEmptyDirectoryFlag && msStatus.isDirectory()) {
@@ -1779,29 +1738,15 @@ public class S3AFileSystem extends FileSystem {
// We have a definitive true / false from MetadataStore, we are done.
return S3AFileStatus.fromFileStatus(msStatus, pm.isEmptyDirectory());
} else {
- DirListingMetadata children = metadataStore.listChildren(path);
- if (children != null) {
- tombstones = children.listTombstones();
- }
LOG.debug("MetadataStore doesn't know if dir is empty, using S3.");
}
} else {
// Either this is not a directory, or we don't care if it is empty
return S3AFileStatus.fromFileStatus(msStatus, pm.isEmptyDirectory());
}
-
- // If the metadata store has no children for it and it's not listed in
- // S3 yet, we'll assume the empty directory is true;
- S3AFileStatus s3FileStatus;
- try {
- s3FileStatus = s3GetFileStatus(path, key, tombstones);
- } catch (FileNotFoundException e) {
- return S3AFileStatus.fromFileStatus(msStatus, Tristate.TRUE);
- }
- return S3Guard.putAndReturn(metadataStore, s3FileStatus, instrumentation);
}
- return S3Guard.putAndReturn(metadataStore,
- s3GetFileStatus(path, key, tombstones), instrumentation);
+ return S3Guard.putAndReturn(metadataStore, s3GetFileStatus(path, key),
+ instrumentation);
}
/**
@@ -1812,8 +1757,8 @@ public class S3AFileSystem extends FileSystem {
* @return Status
* @throws IOException
*/
- private S3AFileStatus s3GetFileStatus(final Path path, String key,
- Set<Path> tombstones) throws IOException {
+ private S3AFileStatus s3GetFileStatus(final Path path, String key)
+ throws IOException {
if (!key.isEmpty()) {
try {
ObjectMetadata meta = getObjectMetadata(key);
@@ -1876,18 +1821,17 @@ public class S3AFileSystem extends FileSystem {
ObjectListing objects = listObjects(request);
- Collection<String> prefixes = objects.getCommonPrefixes();
- Collection<S3ObjectSummary> summaries = objects.getObjectSummaries();
- if (!isEmptyOfKeys(prefixes, tombstones) ||
- !isEmptyOfObjects(summaries, tombstones)) {
+ if (!objects.getCommonPrefixes().isEmpty()
+ || !objects.getObjectSummaries().isEmpty()) {
if (LOG.isDebugEnabled()) {
LOG.debug("Found path as directory (with /): {}/{}",
- prefixes.size(), summaries.size());
+ objects.getCommonPrefixes().size() ,
+ objects.getObjectSummaries().size());
- for (S3ObjectSummary summary : summaries) {
+ for (S3ObjectSummary summary : objects.getObjectSummaries()) {
LOG.debug("Summary: {} {}", summary.getKey(), summary.getSize());
}
- for (String prefix : prefixes) {
+ for (String prefix : objects.getCommonPrefixes()) {
LOG.debug("Prefix: {}", prefix);
}
}
@@ -1909,48 +1853,6 @@ public class S3AFileSystem extends FileSystem {
throw new FileNotFoundException("No such file or directory: " + path);
}
- /** Helper function to determine if a collection of paths is empty
- * after accounting for tombstone markers (if provided).
- * @param keys Collection of path (prefixes / directories or keys).
- * @param tombstones Set of tombstone markers, or null if not applicable.
- * @return false if summaries contains objects not accounted for by
- * tombstones.
- */
- private boolean isEmptyOfKeys(Collection<String> keys, Set<Path>
- tombstones) {
- if (tombstones == null) {
- return keys.isEmpty();
- }
- for (String key : keys) {
- Path qualified = keyToQualifiedPath(key);
- if (!tombstones.contains(qualified)) {
- return false;
- }
- }
- return true;
- }
-
- /** Helper function to determine if a collection of object summaries is empty
- * after accounting for tombstone markers (if provided).
- * @param summaries Collection of objects as returned by listObjects.
- * @param tombstones Set of tombstone markers, or null if not applicable.
- * @return false if summaries contains objects not accounted for by
- * tombstones.
- */
- private boolean isEmptyOfObjects(Collection<S3ObjectSummary> summaries,
- Set<Path> tombstones) {
- if (tombstones == null) {
- return summaries.isEmpty();
- }
- Collection<String> stringCollection = new ArrayList<>(summaries.size());
- for(S3ObjectSummary summary : summaries) {
- stringCollection.add(summary.getKey());
- }
- return isEmptyOfKeys(stringCollection, tombstones);
- }
-
-
-
/**
* Raw version of {@link FileSystem#exists(Path)} which uses S3 only:
* S3Guard MetadataStore, if any, will be skipped.
@@ -1960,7 +1862,7 @@ public class S3AFileSystem extends FileSystem {
Path path = qualify(f);
String key = pathToKey(path);
try {
- return s3GetFileStatus(path, key, null) != null;
+ return s3GetFileStatus(path, key) != null;
} catch (FileNotFoundException e) {
return false;
}
@@ -2518,10 +2420,10 @@ public class S3AFileSystem extends FileSystem {
new Listing.AcceptFilesOnly(qualify(f)));
}
- public RemoteIterator<LocatedFileStatus> listFilesAndEmptyDirectories(Path f,
+ public RemoteIterator<LocatedFileStatus> listFilesAndDirectories(Path f,
boolean recursive) throws IOException {
return innerListFiles(f, recursive,
- new Listing.AcceptAllButS3nDirs());
+ new Listing.AcceptAllButSelfAndS3nDirs(qualify(f)));
}
private RemoteIterator<LocatedFileStatus> innerListFiles(Path f, boolean
@@ -2544,37 +2446,23 @@ public class S3AFileSystem extends FileSystem {
LOG.debug("Requesting all entries under {} with delimiter '{}'",
key, delimiter);
final RemoteIterator<FileStatus> cachedFilesIterator;
- final Set<Path> tombstones;
if (recursive) {
final PathMetadata pm = metadataStore.get(path, true);
- // shouldn't need to check pm.isDeleted() because that will have
- // been caught by getFileStatus above.
- MetadataStoreListFilesIterator metadataStoreListFilesIterator =
- new MetadataStoreListFilesIterator(metadataStore, pm,
- allowAuthoritative);
- tombstones = metadataStoreListFilesIterator.listTombstones();
- cachedFilesIterator = metadataStoreListFilesIterator;
+ cachedFilesIterator = new DescendantsIterator(metadataStore, pm);
} else {
- DirListingMetadata meta = metadataStore.listChildren(path);
- if (meta != null) {
- tombstones = meta.listTombstones();
- } else {
- tombstones = null;
- }
+ final DirListingMetadata meta = metadataStore.listChildren(path);
cachedFilesIterator = listing.createProvidedFileStatusIterator(
S3Guard.dirMetaToStatuses(meta), ACCEPT_ALL, acceptor);
if (allowAuthoritative && meta != null && meta.isAuthoritative()) {
return listing.createLocatedFileStatusIterator(cachedFilesIterator);
}
}
- return listing.createTombstoneReconcilingIterator(
- listing.createLocatedFileStatusIterator(
- listing.createFileStatusListingIterator(path,
- createListObjectsRequest(key, delimiter),
- ACCEPT_ALL,
- acceptor,
- cachedFilesIterator)),
- tombstones);
+ return listing.createLocatedFileStatusIterator(
+ listing.createFileStatusListingIterator(path,
+ createListObjectsRequest(key, delimiter),
+ ACCEPT_ALL,
+ acceptor,
+ cachedFilesIterator));
}
} catch (AmazonClientException e) {
// TODO s3guard:
@@ -2626,7 +2514,7 @@ public class S3AFileSystem extends FileSystem {
final String key = maybeAddTrailingSlash(pathToKey(path));
final Listing.FileStatusAcceptor acceptor =
new Listing.AcceptAllButSelfAndS3nDirs(path);
- DirListingMetadata meta = metadataStore.listChildren(path);
+ final DirListingMetadata meta = metadataStore.listChildren(path);
final RemoteIterator<FileStatus> cachedFileStatusIterator =
listing.createProvidedFileStatusIterator(
S3Guard.dirMetaToStatuses(meta), filter, acceptor);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
index 262a6fa..d008972 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DescendantsIterator.java
@@ -19,7 +19,6 @@
package org.apache.hadoop.fs.s3a.s3guard;
import java.io.IOException;
-import java.util.Collection;
import java.util.LinkedList;
import java.util.NoSuchElementException;
import java.util.Queue;
@@ -104,9 +103,8 @@ public class DescendantsIterator implements RemoteIterator<FileStatus> {
if (meta != null) {
final Path path = meta.getFileStatus().getPath();
if (path.isRoot()) {
- DirListingMetadata rootListing = ms.listChildren(path);
+ final DirListingMetadata rootListing = ms.listChildren(path);
if (rootListing != null) {
- rootListing = rootListing.withoutTombstones();
queue.addAll(rootListing.getListing());
}
} else {
@@ -125,17 +123,11 @@ public class DescendantsIterator implements RemoteIterator<FileStatus> {
if (!hasNext()) {
throw new NoSuchElementException("No more descendants.");
}
- PathMetadata next;
+ final PathMetadata next;
next = queue.poll();
if (next.getFileStatus().isDirectory()) {
final Path path = next.getFileStatus().getPath();
- DirListingMetadata meta = metadataStore.listChildren(path);
- if (meta != null) {
- Collection<PathMetadata> more = meta.withoutTombstones().getListing();
- if (!more.isEmpty()) {
- queue.addAll(more);
- }
- }
+ queue.addAll(metadataStore.listChildren(path).getListing());
}
return next.getFileStatus();
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/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 320fa8d..f13b447 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
@@ -25,12 +25,9 @@ import org.apache.hadoop.fs.Path;
import java.net.URI;
import java.net.URISyntaxException;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashSet;
import java.util.Map;
-import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import com.google.common.base.Preconditions;
@@ -107,26 +104,6 @@ public class DirListingMetadata {
return Collections.unmodifiableCollection(listMap.values());
}
- public Set<Path> listTombstones() {
- Set<Path> tombstones = new HashSet<>();
- for (PathMetadata meta : listMap.values()) {
- if (meta.isDeleted()) {
- tombstones.add(meta.getFileStatus().getPath());
- }
- }
- return tombstones;
- }
-
- public DirListingMetadata withoutTombstones() {
- Collection<PathMetadata> filteredList = new ArrayList<>();
- for (PathMetadata meta : listMap.values()) {
- if (!meta.isDeleted()) {
- filteredList.add(meta);
- }
- }
- return new DirListingMetadata(path, filteredList, isAuthoritative);
- }
-
/**
* @return number of entries tracked. This is not the same as the number
* of entries in the actual directory unless {@link #isAuthoritative()} is
@@ -189,15 +166,6 @@ public class DirListingMetadata {
}
/**
- * Replace an entry with a tombstone.
- * @param childPath path of entry to replace.
- */
- public void markDeleted(Path childPath) {
- checkChildPath(childPath);
- listMap.put(childPath, PathMetadata.tombstone(childPath));
- }
-
- /**
* Remove entry from this directory.
*
* @param childPath path of entry to remove.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
index 784b815..302541c 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
@@ -185,9 +185,6 @@ public class DynamoDBMetadataStore implements MetadataStore {
* DynamoDB. Value is {@value} msec. */
public static final long MIN_RETRY_SLEEP_MSEC = 100;
- private static ValueMap deleteTrackingValueMap =
- new ValueMap().withBoolean(":false", false);
-
private DynamoDB dynamoDB;
private String region;
private Table table;
@@ -289,16 +286,6 @@ public class DynamoDBMetadataStore implements MetadataStore {
@Override
public void delete(Path path) throws IOException {
- innerDelete(path, true);
- }
-
- @Override
- public void forgetMetadata(Path path) throws IOException {
- innerDelete(path, false);
- }
-
- private void innerDelete(Path path, boolean tombstone)
- throws IOException {
path = checkPath(path);
LOG.debug("Deleting from table {} in region {}: {}",
tableName, region, path);
@@ -310,13 +297,7 @@ public class DynamoDBMetadataStore implements MetadataStore {
}
try {
- if (tombstone) {
- Item item = PathMetadataDynamoDBTranslation.pathMetadataToItem(
- PathMetadata.tombstone(path));
- table.putItem(item);
- } else {
- table.deleteItem(pathToKey(path));
- }
+ table.deleteItem(pathToKey(path));
} catch (AmazonClientException e) {
throw translateException("delete", path, e);
}
@@ -329,24 +310,17 @@ public class DynamoDBMetadataStore implements MetadataStore {
tableName, region, path);
final PathMetadata meta = get(path);
- if (meta == null || meta.isDeleted()) {
+ if (meta == null) {
LOG.debug("Subtree path {} does not exist; this will be a no-op", path);
return;
}
for (DescendantsIterator desc = new DescendantsIterator(this, meta);
desc.hasNext();) {
- innerDelete(desc.next().getPath(), true);
+ delete(desc.next().getPath());
}
}
- private Item getConsistentItem(PrimaryKey key) {
- final GetItemSpec spec = new GetItemSpec()
- .withPrimaryKey(key)
- .withConsistentRead(true); // strictly consistent read
- return table.getItem(spec);
- }
-
@Override
public PathMetadata get(Path path) throws IOException {
return get(path, false);
@@ -364,7 +338,10 @@ public class DynamoDBMetadataStore implements MetadataStore {
// Root does not persist in the table
meta = new PathMetadata(makeDirStatus(username, path));
} else {
- final Item item = getConsistentItem(pathToKey(path));
+ final GetItemSpec spec = new GetItemSpec()
+ .withPrimaryKey(pathToKey(path))
+ .withConsistentRead(true); // strictly consistent read
+ final Item item = table.getItem(spec);
meta = itemToPathMetadata(item, username);
LOG.debug("Get from table {} in region {} returning for {}: {}",
tableName, region, path, meta);
@@ -377,8 +354,7 @@ public class DynamoDBMetadataStore implements MetadataStore {
final QuerySpec spec = new QuerySpec()
.withHashKey(pathToParentKeyAttribute(path))
.withConsistentRead(true)
- .withFilterExpression(IS_DELETED + " = :false")
- .withValueMap(deleteTrackingValueMap);
+ .withMaxResultSize(1); // limit 1
final ItemCollection<QueryOutcome> items = table.query(spec);
boolean hasChildren = items.iterator().hasNext();
// When this class has support for authoritative
@@ -422,8 +398,7 @@ public class DynamoDBMetadataStore implements MetadataStore {
final List<PathMetadata> metas = new ArrayList<>();
for (Item item : items) {
- PathMetadata meta = itemToPathMetadata(item, username);
- metas.add(meta);
+ metas.add(itemToPathMetadata(item, username));
}
LOG.trace("Listing table {} in region {} for {} returning {}",
tableName, region, path, metas);
@@ -455,9 +430,9 @@ public class DynamoDBMetadataStore implements MetadataStore {
// Following code is to maintain this invariant by putting all ancestor
// directories of the paths to create.
// ancestor paths that are not explicitly added to paths to create
- Collection<PathMetadata> newItems = new ArrayList<>();
+ Collection<PathMetadata> inferredPathsToCreate = null;
if (pathsToCreate != null) {
- newItems.addAll(pathsToCreate);
+ inferredPathsToCreate = new ArrayList<>(pathsToCreate);
// help set for fast look up; we should avoid putting duplicate paths
final Collection<Path> fullPathsToCreate = new HashSet<>();
for (PathMetadata meta : pathsToCreate) {
@@ -473,20 +448,16 @@ public class DynamoDBMetadataStore implements MetadataStore {
LOG.debug("move: auto-create ancestor path {} for child path {}",
parent, meta.getFileStatus().getPath());
final FileStatus status = makeDirStatus(parent, username);
- newItems.add(new PathMetadata(status, Tristate.FALSE, false));
+ inferredPathsToCreate.add(new PathMetadata(status, Tristate.FALSE));
fullPathsToCreate.add(parent);
parent = parent.getParent();
}
}
}
- if (pathsToDelete != null) {
- for (Path meta : pathsToDelete) {
- newItems.add(PathMetadata.tombstone(meta));
- }
- }
try {
- processBatchWriteRequest(null, pathMetadataToItem(newItems));
+ processBatchWriteRequest(pathToKey(pathsToDelete),
+ pathMetadataToItem(inferredPathsToCreate));
} catch (AmazonClientException e) {
throw translateException("move", (String) null, e);
}
@@ -594,10 +565,13 @@ public class DynamoDBMetadataStore implements MetadataStore {
// first existent ancestor
Path path = meta.getFileStatus().getPath().getParent();
while (path != null && !path.isRoot()) {
- final Item item = getConsistentItem(pathToKey(path));
- if (!itemExists(item)) {
+ final GetItemSpec spec = new GetItemSpec()
+ .withPrimaryKey(pathToKey(path))
+ .withConsistentRead(true); // strictly consistent read
+ final Item item = table.getItem(spec);
+ if (item == null) {
final FileStatus status = makeDirStatus(path, username);
- metasToPut.add(new PathMetadata(status, Tristate.FALSE, false));
+ metasToPut.add(new PathMetadata(status, Tristate.FALSE));
path = path.getParent();
} else {
break;
@@ -606,17 +580,6 @@ public class DynamoDBMetadataStore implements MetadataStore {
return metasToPut;
}
- private boolean itemExists(Item item) {
- if (item == null) {
- return false;
- }
- if (item.hasAttribute(IS_DELETED) &&
- item.getBoolean(IS_DELETED)) {
- return false;
- }
- return true;
- }
-
/** Create a directory FileStatus using current system time as mod time. */
static FileStatus makeDirStatus(Path f, String owner) {
return new FileStatus(0, true, 1, 0, System.currentTimeMillis(), 0,
@@ -628,8 +591,9 @@ public class DynamoDBMetadataStore implements MetadataStore {
LOG.debug("Saving to table {} in region {}: {}", tableName, region, meta);
// directory path
- PathMetadata p = new PathMetadata(makeDirStatus(meta.getPath(), username),
- meta.isEmpty(), false);
+ PathMetadata p = new PathMetadata(
+ makeDirStatus(meta.getPath(), username),
+ meta.isEmpty());
// First add any missing ancestors...
final Collection<PathMetadata> metasToPut = fullPathsToPut(p);
@@ -706,13 +670,13 @@ public class DynamoDBMetadataStore implements MetadataStore {
itemCount++;
if (deletionBatch.size() == S3GUARD_DDB_BATCH_WRITE_REQUEST_LIMIT) {
Thread.sleep(delay);
- processBatchWriteRequest(pathToKey(deletionBatch), null);
+ processBatchWriteRequest(pathToKey(deletionBatch), new Item[0]);
deletionBatch.clear();
}
}
if (deletionBatch.size() > 0) {
Thread.sleep(delay);
- processBatchWriteRequest(pathToKey(deletionBatch), null);
+ processBatchWriteRequest(pathToKey(deletionBatch), new Item[0]);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/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 d7e256d..52e5b2a 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
@@ -101,33 +101,29 @@ public class LocalMetadataStore implements MetadataStore {
}
@Override
- public void delete(Path p) throws IOException {
- doDelete(p, false, true);
- }
-
- @Override
- public void forgetMetadata(Path p) throws IOException {
- doDelete(p, false, false);
+ public void delete(Path path) throws IOException {
+ doDelete(path, false);
}
@Override
public void deleteSubtree(Path path) throws IOException {
- doDelete(path, true, true);
+ doDelete(path, true);
}
- private synchronized void doDelete(Path p, boolean recursive, boolean
- tombstone) {
+ private synchronized void doDelete(Path p, boolean recursive) {
Path path = standardize(p);
+ // We could implement positive hit for 'deleted' files. For now we
+ // do not track them.
// Delete entry from file cache, then from cached parent directory, if any
- deleteHashEntries(path, tombstone);
+ removeHashEntries(path);
if (recursive) {
// Remove all entries that have this dir as path prefix.
- deleteHashByAncestor(path, dirHash, tombstone);
- deleteHashByAncestor(path, fileHash, tombstone);
+ clearHashByAncestor(path, dirHash);
+ clearHashByAncestor(path, fileHash);
}
}
@@ -161,7 +157,7 @@ public class LocalMetadataStore implements MetadataStore {
*/
private Tristate isEmptyDirectory(Path p) {
DirListingMetadata dirMeta = dirHash.get(p);
- return dirMeta.withoutTombstones().isEmpty();
+ return dirMeta.isEmpty();
}
@Override
@@ -190,9 +186,9 @@ public class LocalMetadataStore implements MetadataStore {
synchronized (this) {
// 1. Delete pathsToDelete
- for (Path meta : pathsToDelete) {
- LOG.debug("move: deleting metadata {}", meta);
- delete(meta);
+ for (Path p : pathsToDelete) {
+ LOG.debug("move: deleting metadata {}", p);
+ delete(p);
}
// 2. Create new destination path metadata
@@ -327,25 +323,13 @@ public class LocalMetadataStore implements MetadataStore {
}
@VisibleForTesting
- static <T> void deleteHashByAncestor(Path ancestor, Map<Path, T> hash,
- boolean tombstone) {
+ static <T> void clearHashByAncestor(Path ancestor, 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();
- T meta = entry.getValue();
if (isAncestorOf(ancestor, f)) {
- if (tombstone) {
- if (meta instanceof PathMetadata) {
- entry.setValue((T) PathMetadata.tombstone(f));
- } else if (meta instanceof DirListingMetadata) {
- it.remove();
- } else {
- throw new IllegalStateException("Unknown type in hash");
- }
- } else {
- it.remove();
- }
+ it.remove();
}
}
}
@@ -367,21 +351,16 @@ public class LocalMetadataStore implements MetadataStore {
* Update fileHash and dirHash to reflect deletion of file 'f'. Call with
* lock held.
*/
- private void deleteHashEntries(Path path, boolean tombstone) {
+ private void removeHashEntries(Path path) {
// Remove target file/dir
LOG.debug("delete file entry for {}", path);
- if (tombstone) {
- fileHash.put(path, PathMetadata.tombstone(path));
- } else {
- fileHash.remove(path);
- }
+ fileHash.remove(path);
// Update this and parent dir listing, if any
/* If this path is a dir, remove its listing */
LOG.debug("removing listing of {}", path);
-
dirHash.remove(path);
/* Remove this path from parent's dir listing */
@@ -390,11 +369,7 @@ public class LocalMetadataStore implements MetadataStore {
DirListingMetadata dir = dirHash.get(parent);
if (dir != null) {
LOG.debug("removing parent's entry for {} ", path);
- if (tombstone) {
- dir.markDeleted(path);
- } else {
- dir.remove(path);
- }
+ dir.remove(path);
}
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/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 9502a8a..5511532 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
@@ -22,7 +22,6 @@ import java.io.Closeable;
import java.io.IOException;
import java.util.Collection;
-import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
@@ -56,8 +55,7 @@ public interface MetadataStore extends Closeable {
void initialize(Configuration conf) throws IOException;
/**
- * Deletes exactly one path, leaving a tombstone to prevent lingering,
- * inconsistent copies of it from being listed.
+ * Deletes exactly one path.
*
* @param path the path to delete
* @throws IOException if there is an error
@@ -65,20 +63,7 @@ public interface MetadataStore extends Closeable {
void delete(Path path) throws IOException;
/**
- * Removes the record of exactly one path. Does not leave a tombstone (see
- * {@link MetadataStore#delete(Path)}. It is currently intended for testing
- * only, and a need to use it as part of normal FileSystem usage is not
- * anticipated.
- *
- * @param path the path to delete
- * @throws IOException if there is an error
- */
- @VisibleForTesting
- void forgetMetadata(Path path) throws IOException;
-
- /**
- * Deletes the entire sub-tree rooted at the given path, leaving tombstones
- * to prevent lingering, inconsistent copies of it from being listed.
+ * Deletes the entire sub-tree rooted at the given path.
*
* In addition to affecting future calls to {@link #get(Path)},
* implementations must also update any stored {@code DirListingMetadata}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
deleted file mode 100644
index 272b1f4..0000000
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreListFilesIterator.java
+++ /dev/null
@@ -1,168 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.hadoop.fs.s3a.s3guard;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.Queue;
-import java.util.Set;
-
-import com.google.common.base.Preconditions;
-import org.apache.hadoop.classification.InterfaceAudience;
-import org.apache.hadoop.classification.InterfaceStability;
-import org.apache.hadoop.fs.FileStatus;
-import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.RemoteIterator;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * {@code MetadataStoreListFilesIterator} is a {@link RemoteIterator} that
- * is similar to {@code DescendantsIterator} but does not return directories
- * that have (or may have) children, and will also provide access to the set of
- * tombstones to allow recently deleted S3 objects to be filtered out from a
- * corresponding request. In other words, it returns tombstones and the same
- * set of objects that should exist in S3: empty directories, and files, and not
- * other directories whose existence is inferred therefrom.
- *
- * For example, assume the consistent store contains metadata representing this
- * file system structure:
- *
- * <pre>
- * /dir1
- * |-- dir2
- * | |-- file1
- * | `-- file2
- * `-- dir3
- * |-- dir4
- * | `-- file3
- * |-- dir5
- * | `-- file4
- * `-- dir6
- * </pre>
- *
- * Consider this code sample:
- * <pre>
- * final PathMetadata dir1 = get(new Path("/dir1"));
- * for (MetadataStoreListFilesIterator files =
- * new MetadataStoreListFilesIterator(dir1); files.hasNext(); ) {
- * final FileStatus status = files.next().getFileStatus();
- * System.out.printf("%s %s%n", status.isDirectory() ? 'D' : 'F',
- * status.getPath());
- * }
- * </pre>
- *
- * The output is:
- * <pre>
- * F /dir1/dir2/file1
- * F /dir1/dir2/file2
- * F /dir1/dir3/dir4/file3
- * F /dir1/dir3/dir5/file4
- * D /dir1/dir3/dir6
- * </pre>
- */
-@InterfaceAudience.Private
-@InterfaceStability.Evolving
-public class MetadataStoreListFilesIterator implements
- RemoteIterator<FileStatus> {
- public static final Logger LOG = LoggerFactory.getLogger(
- MetadataStoreListFilesIterator.class);
-
- private final boolean allowAuthoritative;
- private final MetadataStore metadataStore;
- private final Set<Path> tombstones = new HashSet<>();
- private Iterator<FileStatus> leafNodesIterator = null;
-
- public MetadataStoreListFilesIterator(MetadataStore ms, PathMetadata meta,
- boolean allowAuthoritative) throws IOException {
- Preconditions.checkNotNull(ms);
- this.metadataStore = ms;
- this.allowAuthoritative = allowAuthoritative;
- prefetch(meta);
- }
-
- private void prefetch(PathMetadata meta) throws IOException {
- final Queue<PathMetadata> queue = new LinkedList<>();
- final Collection<FileStatus> leafNodes = new ArrayList<>();
-
- if (meta != null) {
- final Path path = meta.getFileStatus().getPath();
- if (path.isRoot()) {
- DirListingMetadata rootListing = metadataStore.listChildren(path);
- if (rootListing != null) {
- tombstones.addAll(rootListing.listTombstones());
- queue.addAll(rootListing.withoutTombstones().getListing());
- }
- } else {
- queue.add(meta);
- }
- }
-
- while(!queue.isEmpty()) {
- PathMetadata nextMetadata = queue.poll();
- FileStatus nextStatus = nextMetadata.getFileStatus();
- if (nextStatus.isFile()) {
- // All files are leaf nodes by definition
- leafNodes.add(nextStatus);
- continue;
- }
- if (nextStatus.isDirectory()) {
- final Path path = nextStatus.getPath();
- DirListingMetadata children = metadataStore.listChildren(path);
- if (children != null) {
- tombstones.addAll(children.listTombstones());
- Collection<PathMetadata> liveChildren =
- children.withoutTombstones().getListing();
- if (!liveChildren.isEmpty()) {
- // If it's a directory, has children, not all deleted, then we
- // add the children to the queue and move on to the next node
- queue.addAll(liveChildren);
- continue;
- } else if (allowAuthoritative && children.isAuthoritative()) {
- leafNodes.add(nextStatus);
- }
- }
- }
- // Directories that *might* be empty are ignored for now, since we
- // cannot confirm that they are empty without incurring other costs.
- // Users of this class can still discover empty directories via S3's
- // fake directories, subject to the same consistency semantics as before.
- // The only other possibility is a symlink, which is unsupported on S3A.
- }
- leafNodesIterator = leafNodes.iterator();
- }
-
- @Override
- public boolean hasNext() {
- return leafNodesIterator.hasNext();
- }
-
- @Override
- public FileStatus next() {
- return leafNodesIterator.next();
- }
-
- public Set<Path> listTombstones() {
- return tombstones;
- }
-}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
index 65019eb..3869d13 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/NullMetadataStore.java
@@ -52,11 +52,6 @@ public class NullMetadataStore implements MetadataStore {
}
@Override
- public void forgetMetadata(Path path) throws IOException {
- return;
- }
-
- @Override
public void deleteSubtree(Path path) throws IOException {
return;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
index d799e16..b5d4f04 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadata.java
@@ -22,7 +22,6 @@ import com.google.common.base.Preconditions;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.FileStatus;
-import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.Tristate;
/**
@@ -35,13 +34,6 @@ public class PathMetadata {
private final FileStatus fileStatus;
private Tristate isEmptyDirectory;
- private boolean isDeleted;
-
- public static PathMetadata tombstone(Path path) {
- long now = System.currentTimeMillis();
- FileStatus status = new FileStatus(0, false, 0, 0, now, path);
- return new PathMetadata(status, Tristate.UNKNOWN, true);
- }
/**
* Creates a new {@code PathMetadata} containing given {@code FileStatus}.
@@ -52,11 +44,6 @@ public class PathMetadata {
}
public PathMetadata(FileStatus fileStatus, Tristate isEmptyDir) {
- this(fileStatus, isEmptyDir, false);
- }
-
- public PathMetadata(FileStatus fileStatus, Tristate isEmptyDir, boolean
- isDeleted) {
Preconditions.checkNotNull(fileStatus, "fileStatus must be non-null");
Preconditions.checkNotNull(fileStatus.getPath(), "fileStatus path must be" +
" non-null");
@@ -64,7 +51,6 @@ public class PathMetadata {
" be absolute");
this.fileStatus = fileStatus;
this.isEmptyDirectory = isEmptyDir;
- this.isDeleted = isDeleted;
}
/**
@@ -87,14 +73,6 @@ public class PathMetadata {
this.isEmptyDirectory = isEmptyDirectory;
}
- public boolean isDeleted() {
- return isDeleted;
- }
-
- void setIsDeleted(boolean isDeleted) {
- this.isDeleted = isDeleted;
- }
-
@Override
public boolean equals(Object o) {
if (!(o instanceof PathMetadata)) {
@@ -113,7 +91,6 @@ public class PathMetadata {
return "PathMetadata{" +
"fileStatus=" + fileStatus +
"; isEmptyDirectory=" + isEmptyDirectory +
- "; isDeleted=" + isDeleted +
'}';
}
@@ -122,10 +99,10 @@ public class PathMetadata {
* @param sb target StringBuilder
*/
public void prettyPrint(StringBuilder sb) {
- sb.append(String.format("%-5s %-20s %-7d %-8s %-6s",
+ sb.append(String.format("%-5s %-20s %-7d %s",
fileStatus.isDirectory() ? "dir" : "file",
fileStatus.getPath().toString(), fileStatus.getLen(),
- isEmptyDirectory.name(), isDeleted));
+ isEmptyDirectory.name()));
sb.append(fileStatus);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
index c206a00..1c04016 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathMetadataDynamoDBTranslation.java
@@ -39,7 +39,6 @@ import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.s3a.Constants;
-import org.apache.hadoop.fs.s3a.Tristate;
/**
* Defines methods for translating between domain model objects and their
@@ -63,7 +62,6 @@ final class PathMetadataDynamoDBTranslation {
static final String FILE_LENGTH = "file_length";
@VisibleForTesting
static final String BLOCK_SIZE = "block_size";
- static final String IS_DELETED = "is_deleted";
/** Table version field {@value} in version marker item. */
@VisibleForTesting
@@ -135,10 +133,8 @@ final class PathMetadataDynamoDBTranslation {
fileStatus = new FileStatus(len, false, 1, block, modTime, 0, null,
username, username, path);
}
- boolean isDeleted =
- item.hasAttribute(IS_DELETED) ? item.getBoolean(IS_DELETED) : false;
- return new PathMetadata(fileStatus, Tristate.UNKNOWN, isDeleted);
+ return new PathMetadata(fileStatus);
}
/**
@@ -158,7 +154,6 @@ final class PathMetadataDynamoDBTranslation {
.withLong(MOD_TIME, status.getModificationTime())
.withLong(BLOCK_SIZE, status.getBlockSize());
}
- item.withBoolean(IS_DELETED, meta.isDeleted());
return item;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
index 19f0e50..53dc991 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
@@ -39,7 +39,6 @@ import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import java.util.Set;
import static org.apache.hadoop.fs.s3a.Constants.*;
import static org.apache.hadoop.fs.s3a.Statistic.*;
@@ -144,8 +143,7 @@ public final class S3Guard {
/**
* Convert the data of a directory listing to an array of {@link FileStatus}
- * entries. Tombstones are filtered out at this point. If the listing is null
- * an empty array is returned.
+ * entries. If the listing is null an empty array is returned.
* @param dirMeta directory listing -may be null
* @return a possibly-empty array of file status entries
*/
@@ -155,15 +153,15 @@ public final class S3Guard {
}
Collection<PathMetadata> listing = dirMeta.getListing();
- List<FileStatus> statuses = new ArrayList<>();
+ FileStatus[] statuses = new FileStatus[listing.size()];
+ // HADOOP-13760: filter out deleted entries here
+ int i = 0;
for (PathMetadata pm : listing) {
- if (!pm.isDeleted()) {
- statuses.add(pm.getFileStatus());
- }
+ statuses[i++] = pm.getFileStatus();
}
- return statuses.toArray(new FileStatus[0]);
+ return statuses;
}
/**
@@ -198,8 +196,6 @@ public final class S3Guard {
false);
}
- Set<Path> deleted = dirMeta.listTombstones();
-
// Since we treat the MetadataStore as a "fresher" or "consistent" view
// of metadata, we always use its metadata first.
@@ -207,11 +203,10 @@ public final class S3Guard {
// we will basically start with the set of directory entries in the
// DirListingMetadata, and add any that only exist in the backingStatuses.
+ // HADOOP-13760: filter out deleted files via PathMetadata#isDeleted() here
+
boolean changed = false;
for (FileStatus s : backingStatuses) {
- if (deleted.contains(s.getPath())) {
- continue;
- }
// Minor race condition here. Multiple threads could add to this
// mutable DirListingMetadata. Since it is backed by a
@@ -250,7 +245,7 @@ public final class S3Guard {
* list will contain [/a/b/c/d, /a/b/c, /a/b]. /a/b/c/d is
* an empty, dir, and the other dirs only contain their child
* dir.
- * @param owner Hadoop user name.
+ * @param owner Hadoop user name
*/
public static void makeDirsOrdered(MetadataStore ms, List<Path> dirs,
String owner) {
@@ -285,8 +280,7 @@ public final class S3Guard {
children = new ArrayList<>(1);
children.add(new PathMetadata(prevStatus));
}
- DirListingMetadata dirMeta =
- new DirListingMetadata(f, children, true);
+ DirListingMetadata dirMeta = new DirListingMetadata(f, children, true);
try {
ms.put(dirMeta);
ms.put(new PathMetadata(status));
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
index 9bd0cb8..3d4f11d 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
@@ -446,7 +446,7 @@ public abstract class S3GuardTool extends Configured implements Tool {
private long importDir(FileStatus status) throws IOException {
Preconditions.checkArgument(status.isDirectory());
RemoteIterator<LocatedFileStatus> it =
- s3a.listFilesAndEmptyDirectories(status.getPath(), true);
+ s3a.listFilesAndDirectories(status.getPath(), true);
long items = 0;
while (it.hasNext()) {
@@ -686,8 +686,7 @@ public abstract class S3GuardTool extends Configured implements Tool {
} catch (FileNotFoundException e) {
}
PathMetadata meta = ms.get(qualified);
- FileStatus msStatus = (meta != null && !meta.isDeleted()) ?
- meta.getFileStatus() : null;
+ FileStatus msStatus = meta != null ? meta.getFileStatus() : null;
compareDir(msStatus, s3Status, out);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
index fb6e370..55e310b 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardEmptyDirs.java
@@ -39,47 +39,45 @@ public class ITestS3GuardEmptyDirs extends AbstractS3ATestBase {
@Test
public void testEmptyDirs() throws Exception {
+
S3AFileSystem fs = getFileSystem();
Assume.assumeTrue(fs.hasMetadataStore());
MetadataStore configuredMs = fs.getMetadataStore();
+
+ // 1. Simulate files already existing in the bucket before we started our
+ // cluster. Temporarily disable the MetadataStore so it doesn't witness
+ // us creating these files.
+
+ fs.setMetadataStore(new NullMetadataStore());
+
Path existingDir = path("existing-dir");
+ assertTrue(fs.mkdirs(existingDir));
Path existingFile = path("existing-dir/existing-file");
- try {
- // 1. Simulate files already existing in the bucket before we started our
- // cluster. Temporarily disable the MetadataStore so it doesn't witness
- // us creating these files.
-
- fs.setMetadataStore(new NullMetadataStore());
- assertTrue(fs.mkdirs(existingDir));
- touch(fs, existingFile);
+ touch(fs, existingFile);
- // 2. Simulate (from MetadataStore's perspective) starting our cluster and
- // creating a file in an existing directory.
- fs.setMetadataStore(configuredMs); // "start cluster"
- Path newFile = path("existing-dir/new-file");
- touch(fs, newFile);
+ // 2. Simulate (from MetadataStore's perspective) starting our cluster and
+ // creating a file in an existing directory.
+ fs.setMetadataStore(configuredMs); // "start cluster"
+ Path newFile = path("existing-dir/new-file");
+ touch(fs, newFile);
- S3AFileStatus status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should not be empty dir", Tristate.FALSE,
- status.isEmptyDirectory());
+ S3AFileStatus status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should not be empty dir", Tristate.FALSE,
+ status.isEmptyDirectory());
- // 3. Assert that removing the only file the MetadataStore witnessed
- // being created doesn't cause it to think the directory is now empty.
- fs.delete(newFile, false);
- status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should not be empty dir", Tristate.FALSE,
- status.isEmptyDirectory());
+ // 3. Assert that removing the only file the MetadataStore witnessed
+ // being created doesn't cause it to think the directory is now empty.
+ fs.delete(newFile, false);
+ status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should not be empty dir", Tristate.FALSE,
+ status.isEmptyDirectory());
- // 4. Assert that removing the final file, that existed "before"
- // MetadataStore started, *does* cause the directory to be marked empty.
- fs.delete(existingFile, false);
- status = fs.innerGetFileStatus(existingDir, true);
- assertEquals("Should be empty dir now", Tristate.TRUE,
- status.isEmptyDirectory());
- } finally {
- configuredMs.forgetMetadata(existingFile);
- configuredMs.forgetMetadata(existingDir);
- }
+ // 4. Assert that removing the final file, that existed "before"
+ // MetadataStore started, *does* cause the directory to be marked empty.
+ fs.delete(existingFile, false);
+ status = fs.innerGetFileStatus(existingDir, true);
+ assertEquals("Should be empty dir now", Tristate.TRUE,
+ status.isEmptyDirectory());
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/c1775960/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
index 8771fd2..5e83906 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java
@@ -56,185 +56,8 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
return new S3AContract(conf);
}
- /**
- * Helper function for other test cases: does a single rename operation and
- * validates the aftermath.
- * @param mkdirs Directories to create
- * @param srcdirs Source paths for rename operation
- * @param dstdirs Destination paths for rename operation
- * @param yesdirs Files that must exist post-rename (e.g. srcdirs children)
- * @param nodirs Files that must not exist post-rename (e.g. dstdirs children)
- * @throws Exception
- */
- private void doTestRenameSequence(Path[] mkdirs, Path[] srcdirs,
- Path[] dstdirs, Path[] yesdirs, Path[] nodirs) throws Exception {
- S3AFileSystem fs = getFileSystem();
- Assume.assumeTrue(fs.hasMetadataStore());
-
- if (mkdirs != null) {
- for (Path mkdir : mkdirs) {
- assertTrue(fs.mkdirs(mkdir));
- }
- Thread.sleep(InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
- }
-
- assertTrue("srcdirs and dstdirs must have equal length",
- srcdirs.length == dstdirs.length);
- for (int i = 0; i < srcdirs.length; i++) {
- assertTrue("Rename returned false: " + srcdirs[i] + " -> " + dstdirs[i],
- fs.rename(srcdirs[i], dstdirs[i]));
- }
-
- for (Path yesdir : yesdirs) {
- assertTrue("Path was supposed to exist: " + yesdir, fs.exists(yesdir));
- }
- for (Path nodir : nodirs) {
- assertFalse("Path is not supposed to exist: " + nodir, fs.exists(nodir));
- }
- }
-
- /**
- * Tests that after renaming a directory, the original directory and its
- * contents are indeed missing and the corresponding new paths are visible.
- * @throws Exception
- */
- @Test
- public void testConsistentListAfterRename() throws Exception {
- Path[] mkdirs = {
- path("d1/f"),
- path("d1/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)
- };
- Path[] srcdirs = {path("d1")};
- Path[] dstdirs = {path("d2")};
- Path[] yesdirs = {path("d2"), path("d2/f"),
- path("d2/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)};
- Path[] nodirs = {path("d1"), path("d1/f"),
- path("d1/f" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)};
- doTestRenameSequence(mkdirs, srcdirs, dstdirs, yesdirs, nodirs);
- getFileSystem().delete(path("d1"), true);
- getFileSystem().delete(path("d2"), true);
- }
-
- /**
- * Tests a circular sequence of renames to verify that overwriting recently
- * deleted files and reading recently created files from rename operations
- * works as expected.
- * @throws Exception
- */
- @Test
- public void testRollingRenames() throws Exception {
- Path[] dir0 = {path("rolling/1")};
- Path[] dir1 = {path("rolling/2")};
- Path[] dir2 = {path("rolling/3")};
- // These sets have to be in reverse order compared to the movement
- Path[] setA = {dir1[0], dir0[0]};
- Path[] setB = {dir2[0], dir1[0]};
- Path[] setC = {dir0[0], dir2[0]};
-
- for(int i = 0; i < 2; i++) {
- Path[] firstSet = i == 0 ? setA : null;
- doTestRenameSequence(firstSet, setA, setB, setB, dir0);
- doTestRenameSequence(null, setB, setC, setC, dir1);
- doTestRenameSequence(null, setC, setA, setA, dir2);
- }
-
- S3AFileSystem fs = getFileSystem();
- assertFalse("Renaming deleted file should have failed",
- fs.rename(dir2[0], dir1[0]));
- assertTrue("Renaming over existing file should have succeeded",
- fs.rename(dir1[0], dir0[0]));
- }
-
- /**
- * Tests that deleted files immediately stop manifesting in list operations
- * even when the effect in S3 is delayed.
- * @throws Exception
- */
- @Test
- public void testConsistentListAfterDelete() throws Exception {
- S3AFileSystem fs = getFileSystem();
- // test will fail if NullMetadataStore (the default) is configured: skip it.
- Assume.assumeTrue(fs.hasMetadataStore());
-
- // Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
- // in listObjects() results via InconsistentS3Client
- Path inconsistentPath =
- path("a/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING);
-
- Path[] testDirs = {path("a/b/dir1"),
- path("a/b/dir2"),
- inconsistentPath};
-
- for (Path path : testDirs) {
- assertTrue(fs.mkdirs(path));
- }
- Thread.sleep(2 * InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
- for (Path path : testDirs) {
- assertTrue(fs.delete(path, false));
- }
-
- FileStatus[] paths = fs.listStatus(path("a/b/"));
- List<Path> list = new ArrayList<>();
- for (FileStatus fileState : paths) {
- list.add(fileState.getPath());
- }
- assertFalse(list.contains(path("a/b/dir1")));
- assertFalse(list.contains(path("a/b/dir2")));
- // This should fail without S3Guard, and succeed with it.
- assertFalse(list.contains(inconsistentPath));
- }
-
- /**
- * Tests that rename immediately after files in the source directory are
- * deleted results in exactly the correct set of destination files and none
- * of the source files.
- * @throws Exception
- */
@Test
- public void testConsistentRenameAfterDelete() throws Exception {
- S3AFileSystem fs = getFileSystem();
- // test will fail if NullMetadataStore (the default) is configured: skip it.
- Assume.assumeTrue(fs.hasMetadataStore());
-
- // Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
- // in listObjects() results via InconsistentS3Client
- Path inconsistentPath =
- path("a/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING);
-
- Path[] testDirs = {path("a/b/dir1"),
- path("a/b/dir2"),
- inconsistentPath};
-
- for (Path path : testDirs) {
- assertTrue(fs.mkdirs(path));
- }
- Thread.sleep(2 * InconsistentAmazonS3Client.DELAY_KEY_MILLIS);
- assertTrue(fs.delete(testDirs[1], false));
- assertTrue(fs.delete(testDirs[2], false));
-
- fs.rename(path("a"), path("a3"));
- FileStatus[] paths = fs.listStatus(path("a3/b"));
- List<Path> list = new ArrayList<>();
- for (FileStatus fileState : paths) {
- list.add(fileState.getPath());
- }
- assertTrue(list.contains(path("a3/b/dir1")));
- assertFalse(list.contains(path("a3/b/dir2")));
- // This should fail without S3Guard, and succeed with it.
- assertFalse(list.contains(
- path("a3/b/dir3-" + InconsistentAmazonS3Client.DELAY_KEY_SUBSTRING)));
-
- try {
- RemoteIterator<LocatedFileStatus> old = fs.listFilesAndEmptyDirectories(
- path("a"), true);
- fail("Recently renamed dir should not be visible");
- } catch(FileNotFoundException e) {
- // expected
- }
- }
-
- @Test
- public void testConsistentListStatusAfterPut() throws Exception {
+ public void testConsistentListStatus() throws Exception {
S3AFileSystem fs = getFileSystem();
@@ -267,25 +90,23 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
}
/**
- * Similar to {@link #testConsistentListStatusAfterPut()}, this tests that the
- * FS listLocatedStatus() call will return consistent list.
+ * Similar to {@link #testConsistentListStatus()}, this tests that the FS
+ * listLocatedStatus() call will return consistent list.
*/
@Test
- public void testConsistentListLocatedStatusAfterPut() throws Exception {
+ public void testConsistentListLocatedStatus() throws Exception {
final S3AFileSystem fs = getFileSystem();
// This test will fail if NullMetadataStore (the default) is configured:
// skip it.
Assume.assumeTrue(fs.hasMetadataStore());
- String rootDir = "doTestConsistentListLocatedStatusAfterPut";
- fs.mkdirs(path(rootDir));
+ fs.mkdirs(path("doTestConsistentListLocatedStatus"));
final int[] numOfPaths = {0, 1, 10};
for (int normalPathNum : numOfPaths) {
for (int delayedPathNum : numOfPaths) {
LOG.info("Testing with normalPathNum={}, delayedPathNum={}",
normalPathNum, delayedPathNum);
- doTestConsistentListLocatedStatusAfterPut(fs, rootDir, normalPathNum,
- delayedPathNum);
+ doTestConsistentListLocatedStatus(fs, normalPathNum, delayedPathNum);
}
}
}
@@ -297,18 +118,18 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
* @param delayedPathNum number paths listed with delaying
* @throws Exception
*/
- private void doTestConsistentListLocatedStatusAfterPut(S3AFileSystem fs,
- String rootDir, int normalPathNum, int delayedPathNum) throws Exception {
+ private void doTestConsistentListLocatedStatus(S3AFileSystem fs,
+ int normalPathNum, int delayedPathNum) throws Exception {
final List<Path> testDirs = new ArrayList<>(normalPathNum + delayedPathNum);
int index = 0;
for (; index < normalPathNum; index++) {
- testDirs.add(path(rootDir + "/dir-" +
- index));
+ testDirs.add(path("doTestConsistentListLocatedStatus/dir-" + index));
}
for (; index < normalPathNum + delayedPathNum; index++) {
// Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed
// in listObjects() results via InconsistentS3Client
- testDirs.add(path(rootDir + "/dir-" + index + DELAY_KEY_SUBSTRING));
+ testDirs.add(path("doTestConsistentListLocatedStatus/dir-" + index
+ + DELAY_KEY_SUBSTRING));
}
for (Path path : testDirs) {
@@ -320,7 +141,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
// this should return the union data from S3 and MetadataStore
final RemoteIterator<LocatedFileStatus> statusIterator =
- fs.listLocatedStatus(path(rootDir + "/"));
+ fs.listLocatedStatus(path("doTestConsistentListLocatedStatus/"));
List<Path> list = new ArrayList<>();
for (; statusIterator.hasNext();) {
list.add(statusIterator.next().getPath());
@@ -402,13 +223,10 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
fileNames.add("file-" + index + "-" + DELAY_KEY_SUBSTRING);
}
- int filesAndEmptyDirectories = 0;
-
// create files under each test directory
for (Path dir : testDirs) {
for (String fileName : fileNames) {
writeTextFile(fs, new Path(dir, fileName), "I, " + fileName, false);
- filesAndEmptyDirectories++;
}
}
@@ -433,7 +251,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
verifyFileIsListed(listedFiles, baseTestDir, fileNames);
} else {
assertEquals("Unexpected number of files returned by listFiles() call",
- filesAndEmptyDirectories,
+ testDirs.size() * (normalFileNum + delayedFileNum),
listedFiles.size());
for (Path dir : testDirs) {
verifyFileIsListed(listedFiles, dir, fileNames);
@@ -545,11 +363,6 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase {
assertEquals("Unexpected number of results from metadata store. "
+ "Should have /OnS3 and /OnS3AndMS: " + mdResults,
2, mdResults.numEntries());
-
- // If we don't clean this up, the next test run will fail because it will
- // have recorded /OnS3 being deleted even after it's written to noS3Guard.
- getFileSystem().getMetadataStore().forgetMetadata(
- new Path(directory, "OnS3"));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org