cryptoe commented on code in PR #13960:
URL: https://github.com/apache/druid/pull/13960#discussion_r1146292211
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/S3StorageConnector.java:
##########
@@ -237,71 +266,95 @@ public OutputStream write(String path) throws IOException
}
@Override
- public void deleteFile(String path)
+ public void deleteFile(String path) throws IOException
{
- s3Client.deleteObject(config.getBucket(), objectPath(path));
+ try {
+ S3Utils.retryS3Operation(() -> {
+ s3Client.deleteObject(config.getBucket(), objectPath(path));
+ return null;
+ }, config.getMaxRetry());
+ }
+ catch (Exception e) {
+ log.error("Error occurred while deleting file at path [%s]. Error:
[%s]", path, e.getMessage());
+ throw new IOException(e);
+ }
}
@Override
- public void deleteRecursively(String dirName)
+ public void deleteFiles(Iterable<String> paths) throws IOException
{
- ListObjectsV2Request listObjectsRequest = new ListObjectsV2Request()
- .withBucketName(config.getBucket())
- .withPrefix(objectPath(dirName));
- ListObjectsV2Result objectListing =
s3Client.listObjectsV2(listObjectsRequest);
-
- while (objectListing.getObjectSummaries().size() > 0) {
- List<DeleteObjectsRequest.KeyVersion> deleteObjectsRequestKeys =
objectListing.getObjectSummaries()
-
.stream()
-
.map(S3ObjectSummary::getKey)
-
.map(DeleteObjectsRequest.KeyVersion::new)
-
.collect(Collectors.toList());
- DeleteObjectsRequest deleteObjectsRequest = new
DeleteObjectsRequest(config.getBucket()).withKeys(
- deleteObjectsRequestKeys);
- s3Client.deleteObjects(deleteObjectsRequest);
+ int currentItemSize = 0;
+ List<DeleteObjectsRequest.KeyVersion> versions = new ArrayList<>();
- // If the listing is truncated, all S3 objects have been deleted,
otherwise, fetch more using the continuation token
- if (objectListing.isTruncated()) {
-
listObjectsRequest.withContinuationToken(objectListing.getContinuationToken());
- objectListing = s3Client.listObjectsV2(listObjectsRequest);
- } else {
- break;
+ for (String path : paths) {
+ // appending base path to each path
+ versions.add(new DeleteObjectsRequest.KeyVersion(objectPath(path)));
+ currentItemSize++;
+ if (currentItemSize == MAX_NUMBER_OF_LISTINGS) {
+ deleteKeys(versions);
+ // resetting trackers
+ versions.clear();
+ currentItemSize = 0;
}
}
+ // deleting remaining elements
+ if (currentItemSize != 0) {
+ deleteKeys(versions);
+ }
}
- @Override
- public List<String> listDir(String dirName)
+ private void deleteKeys(List<DeleteObjectsRequest.KeyVersion> versions)
throws IOException
Review Comment:
I prefer to keep the names free from retry since that is more of an
implementation detail.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
