TSFenwick commented on code in PR #14131:
URL: https://github.com/apache/druid/pull/14131#discussion_r1212049428
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java:
##########
@@ -64,13 +73,69 @@ public S3DataSegmentKiller(
this.inputDataConfig = inputDataConfig;
}
+ @Override
+ public void kill(List<DataSegment> segments) throws SegmentLoadingException
+ {
+ int size = segments.size();
+ if (size == 0) {
+ return;
+ }
+ if (segments.size() == 1) {
+ kill(segments.get(0));
+ return;
+
+ }
+
+ // we can assume that all segments are in the same bucket.
+ String s3Bucket = MapUtils.getString(segments.get(0).getLoadSpec(),
BUCKET);
+ final ServerSideEncryptingAmazonS3 s3Client = this.s3ClientSupplier.get();
+
+ List<DeleteObjectsRequest.KeyVersion> keysToDelete = segments.stream()
+ .map(segment -> MapUtils.getString(segment.getLoadSpec(), KEY))
+ .flatMap(path -> Stream.of(new
DeleteObjectsRequest.KeyVersion(path),
+ new
DeleteObjectsRequest.KeyVersion(DataSegmentKiller.descriptorPath(path))))
+ .collect(Collectors.toList());
+
+ // max delete object request size is 1000 for S3
+ List<List<DeleteObjectsRequest.KeyVersion>> keysChunks =
Lists.partition(keysToDelete, 1000);
+ DeleteObjectsRequest deleteObjectsRequest = new
DeleteObjectsRequest(s3Bucket);
+ // only return objects failed to delete.
+ deleteObjectsRequest.setQuiet(true);
+
+ List<String> keysNotDeleted = new ArrayList<>();
+ for (List<DeleteObjectsRequest.KeyVersion> keysChunk : keysChunks) {
+ List<String> keysToDeleteStrings = keysChunk.stream().map(
+
DeleteObjectsRequest.KeyVersion::getKey).collect(Collectors.toList());
+ try {
+ deleteObjectsRequest.setKeys(keysChunk);
+ log.info("Removing from bucket: [%s] the following index files: [%s]
from s3!", s3Bucket, keysToDeleteStrings);
+ s3Client.deleteObjects(deleteObjectsRequest);
Review Comment:
@rohangarg i think the current state of this pr is good enough as it is
since it is a major improvement to how it used to run. I dont want perfect to
be the enemy of better. So i would prefer to get this merged and then come back
to this later to make the suggested improvements
--
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]
