Copilot commented on code in PR #15473:
URL: https://github.com/apache/pinot/pull/15473#discussion_r2032528102
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3PinotFS.java:
##########
@@ -390,6 +395,73 @@ public boolean mkdir(URI uri)
}
}
+ @Override
+ public boolean deleteBatch(List<URI> segmentUris, boolean forceDelete)
+ throws IOException {
+ // assumption: all URIs in the batch are from the same bucket
+ boolean deletionResult = true;
+ try {
+ List<ObjectIdentifier> objectsToDelete = new ArrayList<>();
+ LOGGER.info("Deleting URIs {} force {}", segmentUris, forceDelete);
+ for (URI segmentUri : segmentUris) {
+
+ if (isDirectory(segmentUri)) {
+ if (!forceDelete) {
+ Preconditions.checkState(isEmptyDirectory(segmentUri),
+ "ForceDelete flag is not set and directory '%s' is not empty",
segmentUri);
+ }
+
+ // Recursively list files in the directory
+ LOGGER.info("Recursively deleting files in directory {}",
segmentUri);
+ List<URI> filesInDir = listFiles(segmentUri);
+ deletionResult &= deleteBatch(filesInDir, forceDelete);
+ } else {
+ String key = sanitizePath(segmentUri.getPath());
+ objectsToDelete.add(ObjectIdentifier.builder().key(key).build());
+
+ // If batch reaches max size, process the batch
+ if (objectsToDelete.size() >= DELETE_BATCH_SIZE) {
+ deletionResult &= processBatch(segmentUri.getHost(),
objectsToDelete);
+ objectsToDelete.clear(); // Clear list for the next batch
+ }
+ }
+ }
+
+ // Process remaining files in the last batch
+ if (!objectsToDelete.isEmpty()) {
+ deletionResult &= processBatch(segmentUris.get(0).getHost(),
objectsToDelete);
Review Comment:
The method assumes all URIs in the batch belong to the same bucket by using
the host from the first URI. Consider adding a validation check to ensure all
URIs share the same bucket or handle cases where they differ.
--
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]