steveloughran commented on a change in pull request #843: HADOOP-15183 S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/843#discussion_r292657344
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -2016,22 +2264,116 @@ void removeKeys(List<DeleteObjectsRequest.KeyVersion>
keysToDelete,
for (DeleteObjectsRequest.KeyVersion keyVersion : keysToDelete) {
blockRootDelete(keyVersion.getKey());
}
- if (enableMultiObjectsDelete) {
- deleteObjects(new DeleteObjectsRequest(bucket)
- .withKeys(keysToDelete)
- .withQuiet(true));
- } else {
- for (DeleteObjectsRequest.KeyVersion keyVersion : keysToDelete) {
- deleteObject(keyVersion.getKey());
+ try {
+ if (enableMultiObjectsDelete) {
+ deleteObjects(new DeleteObjectsRequest(bucket)
+ .withKeys(keysToDelete)
+ .withQuiet(true));
+ } else {
+ for (DeleteObjectsRequest.KeyVersion keyVersion : keysToDelete) {
+ deleteObject(keyVersion.getKey());
+ }
}
+ } catch (MultiObjectDeleteException ex) {
+ // partial delete.
+ // Update the stats with the count of the actual number of successful
+ // deletions.
+ int rejected = ex.getErrors().size();
+ noteDeleted(keysToDelete.size() - rejected, deleteFakeDir);
+ incrementStatistic(FILES_DELETE_REJECTED, rejected);
+ throw ex;
}
+ noteDeleted(keysToDelete.size(), deleteFakeDir);
+ }
+
+ /**
+ * Note the deletion of files or fake directories deleted.
+ * @param count count of keys deleted.
+ * @param deleteFakeDir are the deletions fake directories?
+ */
+ private void noteDeleted(final int count, final boolean deleteFakeDir) {
if (!deleteFakeDir) {
- instrumentation.fileDeleted(keysToDelete.size());
+ instrumentation.fileDeleted(count);
} else {
- instrumentation.fakeDirsDeleted(keysToDelete.size());
+ instrumentation.fakeDirsDeleted(count);
}
- if (clearKeys) {
- keysToDelete.clear();
+ }
+
+ /**
+ * Invoke {@link #removeKeysS3(List, boolean)} with handling of
+ * {@code MultiObjectDeleteException} in which S3Guard is updated with all
+ * deleted entries, before the exception is rethrown.
+ *
+ * If an exception is not raised. the metastore is not updated.
+ * @param keysToDelete collection of keys to delete on the s3-backend.
+ * if empty, no request is made of the object store.
+ * @param deleteFakeDir indicates whether this is for deleting fake dirs
+ * @throws InvalidRequestException if the request was rejected due to
+ * a mistaken attempt to delete the root directory.
+ * @throws MultiObjectDeleteException one or more of the keys could not
+ * be deleted in a multiple object delete operation.
+ * @throws AmazonClientException amazon-layer failure.
+ * @throws IOException other IO Exception.
+ */
+ @VisibleForTesting
+ @Retries.RetryMixed
+ void removeKeys(
+ final List<DeleteObjectsRequest.KeyVersion> keysToDelete,
+ final boolean deleteFakeDir)
+ throws MultiObjectDeleteException, AmazonClientException,
+ IOException {
+ removeKeys(keysToDelete, deleteFakeDir, new ArrayList<>());
+ }
+
+ /**
+ * Invoke {@link #removeKeysS3(List, boolean)} with handling of
+ * {@code MultiObjectDeleteException} in which S3Guard is updated with all
+ * deleted entries, before the exception is rethrown.
+ *
+ * @param keysToDelete collection of keys to delete on the s3-backend.
+ * if empty, no request is made of the object store.
+ * @param deleteFakeDir indicates whether this is for deleting fake dirs
+ * @param undeletedObjectsOnFailure List which will be built up of all
+ * files that were not deleted. This happens even as an exception
+ * is raised.
+ * @throws InvalidRequestException if the request was rejected due to
+ * a mistaken attempt to delete the root directory.
+ * @throws MultiObjectDeleteException one or more of the keys could not
+ * be deleted in a multiple object delete operation.
+ * @throws AmazonClientException amazon-layer failure.
+ * @throws IOException other IO Exception.
+ */
+ @VisibleForTesting
+ @Retries.RetryMixed
+ void removeKeys(
Review comment:
yes: ITestS3AFailureHandling testMultiObjectDeleteNoPermissions()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
