review feedback

Author: ywangd

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java

@@ -397,21 +397,7 @@ private void deletePartition(OperationPurpose purpose, List<String> partition, D
             } catch (MultiObjectDeleteException e) {
                 // We are sending quiet mode requests so we can't use the deleted keys entry on the exception and instead
                 // first remove all keys that were sent in the request and then add back those that ran into an exception.
-                logger.warn(() -> {
-                    final var sb = new StringBuilder("Failed to delete some blobs ");
-                    Strings.collectionToDelimitedStringWithLimit(
-                        (Iterable<String>) () -> e.getErrors()
-                            .stream()
-                            .map(err -> "[" + err.getKey() + "][" + err.getCode() + "][" + err.getMessage() + "]")
-                            .iterator(),
-                        ",",
-                        "",
-                        "",
-                        1000,
-                        sb
-                    );
-                    return sb;
-                }, e);
+                logger.warn(buildDeletionErrorMessage(e), e);
                 deletionExceptions.useOrMaybeSuppress(e);
                 return;
             } catch (AmazonClientException e) {
@@ -434,6 +420,26 @@ private void deletePartition(OperationPurpose purpose, List<String> partition, D
         }
     }
 
+    private String buildDeletionErrorMessage(MultiObjectDeleteException e) {
+        final var sb = new StringBuilder("Failed to delete some blobs ");
+        final var errors = e.getErrors();
+        for (int i = 0; i < errors.size() && i < MAX_DELETE_EXCEPTIONS; i++) {
+            final var err = errors.get(i);
+            sb.append("[").append(err.getKey()).append("][").append(err.getCode()).append("][").append(err.getMessage()).append("]");
+            if (i < errors.size() - 1) {
+                sb.append(",");
+            }
+        }
+        if (errors.size() > MAX_DELETE_EXCEPTIONS) {
+            sb.append("... (")
+                .append(errors.size())
+                .append(" in total, ")
+                .append(errors.size() - MAX_DELETE_EXCEPTIONS)
+                .append(" omitted)");
+        }
+        return sb.toString();
+    }
+
     /**
      * If there are remaining retries, pause for the configured interval then return true
      *

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java

@@ -1113,7 +1113,7 @@ public void testSuppressedDeletionErrorsAreCapped() {
 
     public void testTrimmedLogAndCappedSuppressedErrorOnMultiObjectDeletionException() {
         final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
-        int maxBulkDeleteSize = randomIntBetween(10, 20);
+        int maxBulkDeleteSize = randomIntBetween(10, 30);
         final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, maxBulkDeleteSize);
 
         final Pattern pattern = Pattern.compile("<Key>(.+?)</Key>");
@@ -1129,7 +1129,7 @@ public void testTrimmedLogAndCappedSuppressedErrorOnMultiObjectDeletionException
                     deletes.append("<Error>");
                     deletes.append("<Code>").append(randomAlphaOfLength(10)).append("</Code>");
                     deletes.append("<Key>").append(key).append("</Key>");
-                    deletes.append("<Message>").append(randomAlphaOfLength(200)).append("</Message>");
+                    deletes.append("<Message>").append(randomAlphaOfLength(40)).append("</Message>");
                     deletes.append("</Error>");
                 }
                 deletes.append("</DeleteResult>");
@@ -1150,7 +1150,9 @@ public void testTrimmedLogAndCappedSuppressedErrorOnMultiObjectDeletionException
                     "deletion log",
                     S3BlobStore.class.getCanonicalName(),
                     Level.WARN,
-                    "Failed to delete some blobs [*... (* in total, * omitted)"
+                    blobs.size() > S3BlobStore.MAX_DELETE_EXCEPTIONS
+                        ? "Failed to delete some blobs [*... (* in total, * omitted)"
+                        : "Failed to delete some blobs [*]"
                 )
             );
             var exception = expectThrows(