You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@druid.apache.org by "kgyrtkirk (via GitHub)" <gi...@apache.org> on 2026/04/01 11:09:06 UTC

[PR] feat: use aws-crt by default for async s3 operations (druid)

kgyrtkirk opened a new pull request, #19249:
URL: https://github.com/apache/druid/pull/19249

   I've investigated some S3 related issues which appeared agains MiniIO in a forked version of Druid.
   After reproducing the issue consistently it was clear that disabling `druid.storage.transfer.useTransferManager` fixes the issue - but beyond disabling it I also checked that utilizing the [crt](https://docs.aws.amazon.com/sdkref/latest/guide/common-runtime.html) backed client also works without issues.
   Also with netty even with S3 sometime a similar 400 happened.
   Because my experience was that `crt` just worked more reliably all the time - I think it should be the default.
   
   * adds config option `druid.storage.transfer.asyncHttpClientType`
     * `crt` (default) uses aws crt lib
     * `netty` (old) netty-nio backed client
   * some stability relate scripts/wfs to validate test stability in the future
   
   
   exceptions:
   * for minio were like this ([full logs are here](https://gist.github.com/kgyrtkirk/53f1740a43ce99c504f116c3fd0cb395))
     ```
     java.lang.RuntimeException: java.util.concurrent.CompletionException: software.amazon.awssdk.services.s3.model.S3Exception: (Service: S3, Status Code: 400, Request ID: null) (SDK Attempt Count: 1)
     ```
   * for aws ([full logs are here](https://gist.github.com/kgyrtkirk/b3980d438430f5c65865e93e0dd91ab5))
     ```
     java.lang.RuntimeException: java.util.concurrent.CompletionException: software.amazon.awssdk.services.s3.model.S3Exception: (Service: S3, Status Code: 400, Request ID: C0BFF04967DEC3EB) (SDK Attempt Count: 1)
     ```
   


-- 
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-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] feat: use aws-crt by default for async s3 operations (druid)

Posted by "gianm (via GitHub)" <gi...@apache.org>.

gianm commented on code in PR #19249:
URL: https://github.com/apache/druid/pull/19249#discussion_r3023364914


##########
.github/scripts/run-stability:
##########
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e

Review Comment:
   Please include a comment about what the purpose of this script is.



##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java:
##########
@@ -191,15 +194,17 @@ public ServerSideEncryptingAmazonS3.Builder getServerSideEncryptingAmazonS3Build
     };
 
     // Create async client supplier for S3TransferManager
+    final String asyncHttpClientType = storageConfig.getS3TransferConfig().getAsyncHttpClientType();
+    if (!"crt".equalsIgnoreCase(asyncHttpClientType) && !"netty".equalsIgnoreCase(asyncHttpClientType)) {

Review Comment:
   Rather than hard coding these strings inline, would be nicer to use an enum, or at least a `public static final Set<String>`.



##########
services/src/test/java/org/apache/druid/testing/embedded/EmbeddedDruidServer.java:
##########
@@ -73,8 +73,12 @@ protected EmbeddedDruidServer()
     );
     beforeStartHooks.add(
         (cluster, self) -> {
-          // Add properties for temporary directories used by the servers
-          final String logsDirectory = cluster.getTestFolder().getOrCreateFolder("indexer-logs").getAbsolutePath();
+          // Add properties for temporary directories used by the servers.
+          // If task.logs.dir is set, write indexer logs there so they survive TestFolder cleanup.

Review Comment:
   Why do we want logs to survive TestFolder cleanup? Generally it's good for tests to clean up any files they wrote, otherwise the system can get littered with junk files. If there's a good reason for this the comment should say it.



-- 
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-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org