This is an automated email from the ASF dual-hosted git repository.

gianm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new cb0e388b2ef Only do multipart uploads in RetryableS3OutputStream if 
the bytes to push expand beyond the threshold to do so (#19893)
cb0e388b2ef is described below

commit cb0e388b2ef9a139ac524b537f3dc82ae4e2fc91
Author: Lucas Capistrant <[email protected]>
AuthorDate: Thu Aug 6 01:10:09 2026 -0500

    Only do multipart uploads in RetryableS3OutputStream if the bytes to push 
expand beyond the threshold to do so (#19893)
---
 .../storage/s3/output/RetryableS3OutputStream.java | 98 +++++++++++++++++-----
 .../s3/output/RetryableS3OutputStreamTest.java     | 79 +++++++++++++++++
 2 files changed, 156 insertions(+), 21 deletions(-)

diff --git 
a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/RetryableS3OutputStream.java
 
b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/RetryableS3OutputStream.java
index f3e2075cb53..075f97ed1cc 100644
--- 
a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/RetryableS3OutputStream.java
+++ 
b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/RetryableS3OutputStream.java
@@ -24,6 +24,7 @@ import com.google.common.io.CountingOutputStream;
 import it.unimi.dsi.fastutil.io.FastBufferedOutputStream;
 import org.apache.druid.java.util.common.FileUtils;
 import org.apache.druid.java.util.common.RetryUtils;
+import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.java.util.common.io.Closer;
 import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
@@ -37,6 +38,7 @@ import 
software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
 import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
 import software.amazon.awssdk.services.s3.model.UploadPartResponse;
 
+import javax.annotation.Nullable;
 import java.io.Closeable;
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -56,12 +58,14 @@ import java.util.concurrent.TimeUnit;
  * <ol>
  * <li>When new data is written, it first creates a chunk in local disk.</li>
  * <li>New data is written to the local chunk until it is full.</li>
- * <li>When the chunk is full, it uploads the chunk to s3 using the multipart 
upload API.
- * Since this happens synchronously, {@link #write(byte[], int, int)} can be 
blocked until the upload is done.
+ * <li>When the chunk is full, a multipart upload is started if one is not 
already in progress, and the chunk is
+ * queued for upload as a part. {@link #write(byte[], int, int)} can be 
blocked while the upload queue is saturated.
  * The upload can be retried when it fails with transient errors.</li>
- * <li>Once the upload succeeds, it creates a new chunk and continue.</li>
- * <li>When the stream is closed, it uploads the last chunk and finalize the 
multipart upload.
- * {@link #close()} can be blocked until upload is done.</li>
+ * <li>Once the chunk is queued, it creates a new chunk and continue.</li>
+ * <li>When the stream is closed, what happens depends on whether any part was 
uploaded. A stream that never filled
+ * a chunk is uploaded as a single object, costing one request instead of the 
three a multipart upload needs;
+ * otherwise the last chunk is uploaded and the multipart upload is finalized. 
{@link #close()} can be blocked
+ * until upload is done.</li>
  *   </ol>
  * For compression format support, this output stream supports compression 
formats if they are <i>concatenatable</i>,
  * such as ZIP or GZIP.
@@ -81,10 +85,19 @@ public class RetryableS3OutputStream extends OutputStream
   private final S3OutputConfig config;
   private final ServerSideEncryptingAmazonS3 s3;
   private final String s3Key;
-  private final String uploadId;
   private final File chunkStorePath;
   private final long chunkSize;
 
+  /**
+   * Multipart upload ID, or null while the stream still fits in {@link 
#currentChunk}. A multipart upload costs a
+   * create and a complete request on top of the part uploads themselves, so 
it is only started once a chunk actually
+   * needs pushing; a stream that never fills a chunk is uploaded by {@link 
#close()} as a single putObject.
+   *
+   * @see #initiateMultipartUploadIfNeeded()
+   */
+  @Nullable
+  private String uploadId;
+
   private final byte[] singularBuffer = new byte[1];
 
   // metric
@@ -123,18 +136,7 @@ public class RetryableS3OutputStream extends OutputStream
     this.s3Key = s3Key;
     this.uploadManager = uploadManager;
 
-    final CreateMultipartUploadResponse result;
-    try {
-      CreateMultipartUploadRequest.Builder requestBuilder = 
CreateMultipartUploadRequest.builder()
-          .bucket(config.getBucket())
-          .key(s3Key);
-      result = S3Utils.retryS3Operation(() -> 
s3.createMultipartUpload(requestBuilder), config.getMaxRetry());
-    }
-    catch (Exception e) {
-      throw new IOException("Unable to start multipart upload", e);
-    }
-    this.uploadId = result.uploadId();
-    this.chunkStorePath = new File(config.getTempDir(), uploadId + 
UUID.randomUUID());
+    this.chunkStorePath = new File(config.getTempDir(), 
UUID.randomUUID().toString());
     FileUtils.mkdirp(this.chunkStorePath);
     this.chunkSize = config.getChunkSize();
     this.pushStopwatch = Stopwatch.createStarted();
@@ -196,12 +198,37 @@ public class RetryableS3OutputStream extends OutputStream
     currentChunk.close();
     final Chunk chunk = currentChunk;
     if (chunk.length() > 0) {
+      initiateMultipartUploadIfNeeded();
       futures.add(
           uploadManager.queueChunkForUpload(s3, s3Key, chunk.id, chunk.file, 
uploadId, config)
       );
     }
   }
 
+  /**
+   * Starts the multipart upload the first time a chunk actually needs 
pushing. Called only from
+   * {@link #pushCurrentChunk()}, so a stream that never fills a chunk issues 
no create request at all.
+   */
+  private void initiateMultipartUploadIfNeeded() throws IOException
+  {
+    if (uploadId != null) {
+      return;
+    }
+    try {
+      final CreateMultipartUploadRequest.Builder requestBuilder = 
CreateMultipartUploadRequest.builder()
+          .bucket(config.getBucket())
+          .key(s3Key);
+      final CreateMultipartUploadResponse result = S3Utils.retryS3Operation(
+          () -> s3.createMultipartUpload(requestBuilder),
+          config.getMaxRetry()
+      );
+      uploadId = result.uploadId();
+    }
+    catch (Exception e) {
+      throw new IOException("Unable to start multipart upload", e);
+    }
+  }
+
   @Override
   public void close() throws IOException
   {
@@ -226,19 +253,48 @@ public class RetryableS3OutputStream extends OutputStream
           uploadId
       );
 
-      final ServiceMetricEvent.Builder builder = new 
ServiceMetricEvent.Builder().setDimension("uploadId", uploadId);
+      final ServiceMetricEvent.Builder builder =
+          new ServiceMetricEvent.Builder().setDimension("uploadId", uploadId 
== null ? "none" : uploadId);
       uploadManager.emitMetric(builder.setMetric(METRIC_TOTAL_UPLOAD_TIME, 
totalUploadTimeMillis));
       uploadManager.emitMetric(builder.setMetric(METRIC_TOTAL_UPLOAD_BYTES, 
totalBytesUploaded));
     });
 
     try (Closer ignored = closer) {
       if (!error) {
-        pushCurrentChunk();
-        completeMultipartUpload();
+        if (uploadId == null) {
+          // Everything written fits in the first chunk, so a single putObject 
does the job that a create, an upload
+          // part and a complete would otherwise take.
+          putCurrentChunkAsWholeObject();
+        } else {
+          pushCurrentChunk();
+          completeMultipartUpload();
+        }
       }
     }
   }
 
+  /**
+   * Uploads {@link #currentChunk} as a complete object. Only valid while 
{@link #uploadId} is null, i.e. when no part
+   * has ever been pushed and the chunk therefore holds the entire stream.
+   */
+  private void putCurrentChunkAsWholeObject() throws IOException
+  {
+    currentChunk.close();
+    if (currentChunk.length() == 0) {
+      // Nothing was written, so there is no object to create.
+      return;
+    }
+    try {
+      S3Utils.retryS3Operation(
+          () -> s3.putObject(config.getBucket(), s3Key, currentChunk.file),
+          config.getMaxRetry()
+      );
+    }
+    catch (Exception e) {
+      throw new IOException(StringUtils.format("Unable to upload s3Key[%s]", 
s3Key), e);
+    }
+  }
+
   private void completeMultipartUpload()
   {
     final List<CompletedPart> pushResults = new ArrayList<>();
diff --git 
a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java
 
b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java
index 07274757f09..48d943c2a7d 100644
--- 
a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java
+++ 
b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java
@@ -19,6 +19,7 @@
 
 package org.apache.druid.storage.s3.output;
 
+import com.google.common.collect.ImmutableList;
 import org.apache.druid.java.util.common.FileUtils;
 import org.apache.druid.java.util.common.HumanReadableBytes;
 import org.apache.druid.java.util.common.IOE;
@@ -42,6 +43,7 @@ import 
software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
 import software.amazon.awssdk.services.s3.model.CompletedPart;
 import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
 import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
+import software.amazon.awssdk.services.s3.model.PutObjectResponse;
 import software.amazon.awssdk.services.s3.model.UploadPartRequest;
 import software.amazon.awssdk.services.s3.model.UploadPartResponse;
 
@@ -217,11 +219,71 @@ public class RetryableS3OutputStreamTest
     s3.assertCancelled();
   }
 
+  /**
+   * A multipart upload costs three S3 requests at minimum — create, upload 
part, complete — so a stream small enough
+   * to need only one part should not use one. A task writes one object per 
output partition, and every request lands
+   * on the same key prefix, so the per-object floor sets the burst rate 
against that prefix.
+   */
+  @Test
+  public void testStreamFittingInOneChunkIsUploadedWithASinglePut() throws 
IOException
+  {
+    chunkSize = 10;
+    ByteBuffer bb = ByteBuffer.allocate(Integer.BYTES);
+    try (RetryableS3OutputStream out =
+             new RetryableS3OutputStream(config, s3, path, s3UploadManager)) {
+      bb.putInt(1);
+      out.write(bb.array());
+    }
+
+    Assertions.assertEquals(0, s3.createMultipartUploadCount);
+    Assertions.assertEquals(0, s3.partRequests.size());
+    Assertions.assertNull(s3.completeRequest);
+    Assertions.assertEquals(ImmutableList.of((long) Integer.BYTES), 
s3.putObjectContentLengths);
+  }
+
+  /**
+   * Once a stream outgrows a single chunk it must use multipart, costing one 
create, one request per part, and one
+   * complete.
+   */
+  @Test
+  public void testStreamSpanningMultipleChunksUsesMultipartUpload() throws 
IOException
+  {
+    chunkSize = 10;
+    ByteBuffer bb = ByteBuffer.allocate(Integer.BYTES);
+    try (RetryableS3OutputStream out =
+             new RetryableS3OutputStream(config, s3, path, s3UploadManager)) {
+      for (int i = 0; i < 25; i++) {
+        bb.clear();
+        bb.putInt(i);
+        out.write(bb.array());
+      }
+    }
+
+    Assertions.assertEquals(1, s3.createMultipartUploadCount);
+    Assertions.assertEquals(10, s3.partRequests.size());
+    Assertions.assertEquals(0, s3.putObjectContentLengths.size());
+    s3.assertCompleted(chunkSize, Integer.BYTES * 25);
+  }
+
+  /**
+   * A stream closed without any bytes written produces no object, and should 
reach S3 not at all to do so.
+   */
+  @Test
+  public void testClosingWithoutWritingCreatesNoObject() throws IOException
+  {
+    chunkSize = 10;
+    new RetryableS3OutputStream(config, s3, path, s3UploadManager).close();
+
+    s3.assertNoRequestsIssued();
+  }
+
   private static class TestAmazonS3 extends ServerSideEncryptingAmazonS3
   {
     private final List<UploadPartRequest> partRequests = new ArrayList<>();
+    private final List<Long> putObjectContentLengths = new ArrayList<>();
 
     private int uploadFailuresLeft;
+    private int createMultipartUploadCount = 0;
     private boolean cancelled = false;
     @Nullable
     private CompleteMultipartUploadRequest completeRequest;
@@ -236,11 +298,28 @@ public class RetryableS3OutputStreamTest
     public CreateMultipartUploadResponse 
createMultipartUpload(CreateMultipartUploadRequest.Builder requestBuilder)
         throws SdkClientException
     {
+      ++createMultipartUploadCount;
       return CreateMultipartUploadResponse.builder()
           .uploadId("uploadId")
           .build();
     }
 
+    @Override
+    public PutObjectResponse putObject(String bucket, String key, File file)
+    {
+      putObjectContentLengths.add(file.length());
+      return PutObjectResponse.builder().build();
+    }
+
+    private void assertNoRequestsIssued()
+    {
+      Assertions.assertEquals(0, createMultipartUploadCount);
+      Assertions.assertEquals(0, putObjectContentLengths.size());
+      Assertions.assertEquals(0, partRequests.size());
+      Assertions.assertNull(completeRequest);
+      Assertions.assertFalse(cancelled);
+    }
+
     @Override
     public UploadPartResponse uploadPart(UploadPartRequest.Builder 
requestBuilder, RequestBody requestBody)
         throws SdkClientException


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to