dramaticlly commented on a change in pull request #4052:
URL: https://github.com/apache/iceberg/pull/4052#discussion_r809621260



##########
File path: 
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -208,8 +212,49 @@ public void testClientFactorySerialization() throws 
Exception {
     validateRead(fileIO2);
   }
 
+  @Test
+  public void testDeleteFilesMultipleBatches() throws Exception {
+    S3FileIO s3FileIO = new S3FileIO(clientFactory::s3, 
getDeletionTestProperties());
+    testDeleteFiles(6, s3FileIO);

Review comment:
       nit, maybe I would think about put it like `deletionBatchSize * 2`
   
   same as below 
   - for lessThan, we can do `deletionBatchSize-1`
   - for SingleWithRemainer do `deletionBatchSize+1`
   
   so it shall help with understanding and hopefully more robust once 
`deletionBatchSize` get changed
   

##########
File path: aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
##########
@@ -213,13 +213,20 @@
    * Enables eTag checks for S3 PUT and MULTIPART upload requests.
    */
   public static final String S3_CHECKSUM_ENABLED = "s3.checksum-enabled";
+
+  /**
+   * Sets the size for batch deletion.
+   */

Review comment:
       Nice finding, I would even consider put a comment to reference where is 
250 comes from 
https://github.com/apache/hadoop/commit/56dee667707926f3796c7757be1a133a362f05c9
 until benchmark is completed




-- 
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]

Reply via email to