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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org