You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/11/24 10:18:03 UTC
[GitHub] [hadoop] mukund-thakur commented on a change in pull request #3534: HADOOP-17409. Remove s3guard from S3A module
mukund-thakur commented on a change in pull request #3534:
URL: https://github.com/apache/hadoop/pull/3534#discussion_r754893451
##########
File path: hadoop-common-project/hadoop-common/src/site/markdown/filesystem/introduction.md
##########
@@ -343,7 +343,7 @@ stores pretend that they are a FileSystem, a FileSystem with the same
features and operations as HDFS. This is —ultimately—a pretence:
they have different characteristics and occasionally the illusion fails.
-1. **Consistency**. Object stores are generally *Eventually Consistent*: it
+1. **Consistency**. Object mqy be *Eventually Consistent*: it
Review comment:
nit : typo mqy -> may
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java
##########
@@ -279,82 +205,19 @@ private ObjectListingIterator createObjectListingIterator(
LOG.debug("Requesting all entries under {} with delimiter '{}'",
key, delimiter);
}
- final RemoteIterator<S3AFileStatus> cachedFilesIterator;
- final Set<Path> tombstones;
- boolean allowAuthoritative = listingOperationCallbacks
- .allowAuthoritative(path);
- if (recursive) {
- final PathMetadata pm = getStoreContext()
- .getMetadataStore()
- .get(path, true);
- if (pm != null) {
- if (pm.isDeleted()) {
- OffsetDateTime deletedAt = OffsetDateTime
- .ofInstant(Instant.ofEpochMilli(
- pm.getFileStatus().getModificationTime()),
- ZoneOffset.UTC);
- throw new FileNotFoundException("Path " + path + " is recorded as " +
- "deleted by S3Guard at " + deletedAt);
- }
- }
- MetadataStoreListFilesIterator metadataStoreListFilesIterator =
- new MetadataStoreListFilesIterator(
- getStoreContext().getMetadataStore(),
- pm,
- allowAuthoritative);
- tombstones = metadataStoreListFilesIterator.listTombstones();
- // if all of the below is true
- // - authoritative access is allowed for this metadatastore
- // for this directory,
- // - all the directory listings are authoritative on the client
- // - the caller does not force non-authoritative access
- // return the listing without any further s3 access
- if (!forceNonAuthoritativeMS &&
- allowAuthoritative &&
- metadataStoreListFilesIterator.isRecursivelyAuthoritative()) {
- S3AFileStatus[] statuses = S3AUtils.iteratorToStatuses(
- metadataStoreListFilesIterator, tombstones);
- cachedFilesIterator = createProvidedFileStatusIterator(
- statuses, ACCEPT_ALL, acceptor);
- return createLocatedFileStatusIterator(cachedFilesIterator);
- }
- cachedFilesIterator = metadataStoreListFilesIterator;
- } else {
- DirListingMetadata meta =
- S3Guard.listChildrenWithTtl(
- getStoreContext().getMetadataStore(),
- path,
- listingOperationCallbacks.getUpdatedTtlTimeProvider(),
- allowAuthoritative);
- if (meta != null) {
- tombstones = meta.listTombstones();
- } else {
- tombstones = null;
- }
- cachedFilesIterator = createProvidedFileStatusIterator(
- S3Guard.dirMetaToStatuses(meta), ACCEPT_ALL, acceptor);
- if (allowAuthoritative && meta != null && meta.isAuthoritative()) {
- // metadata listing is authoritative, so return it directly
- return createLocatedFileStatusIterator(cachedFilesIterator);
- }
- }
- return createTombstoneReconcilingIterator(
- createLocatedFileStatusIterator(
- createFileStatusListingIterator(path,
- listingOperationCallbacks
- .createListObjectsRequest(key,
- delimiter,
- span),
- ACCEPT_ALL,
- acceptor,
- cachedFilesIterator,
- span)),
- collectTombstones ? tombstones : null);
+ return createLocatedFileStatusIterator(
Review comment:
Wow. Listing is super simple now. 💯
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -4284,31 +3929,8 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
String action = "copyFile(" + srcKey + ", " + dstKey + ")";
Invoker readInvoker = readContext.getReadInvoker();
- ObjectMetadata srcom;
- try {
- srcom = once(action, srcKey,
- () ->
- getObjectMetadata(srcKey, changeTracker, readInvoker, "copy"));
- } catch (FileNotFoundException e) {
- // if rename fails at this point it means that the expected file was not
- // found.
- // The cause is believed to always be one of
- // - File was deleted since LIST/S3Guard metastore.list.() knew of it.
- // - S3Guard is asking for a specific version and it's been removed by
- // lifecycle rules.
- // - there's a 404 cached in the S3 load balancers.
- LOG.debug("getObjectMetadata({}) failed to find an expected file",
- srcKey, e);
- // We create an exception, but the text depends on the S3Guard state
- String message = hasMetadataStore()
Review comment:
nit: Won't this make the exception returned to client backward incompatible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org