From c5ae94c2b3400f04182559d73ad7caf947e2c57f Mon Sep 17 00:00:00 2001
From: Bin Fan <fanbin103@gmail.com>
Date: Thu, 8 Aug 2024 15:54:29 -0700
Subject: [PATCH] Rename property key alluxio.underfs.s3.threads.max

### What changes are proposed in this pull request?

Rename property key alluxio.underfs.s3.threads.max to alluxio.underfs.s3.connections.max and bump the default number to 1024

### Why are the changes needed?

The property `alluxio.underfs.s3.threads.max` has a misleading name, as it actually represents the size of a connection pool rather than the max number of threads. Consequently, the value of this property is typically much larger (e.g., Trino uses 256 for the equivalent property to ensure sufficient bandwidth when reading from S3).

However, including "threads" in the name causes some users to worry, as they believe it should be calculated based on the number of vCores (as we found in one PoC that users were unwilling to set this value large) .

This commit changes the name but retains the original property name as an alias to maintain backward compatibility.

### Does this PR introduce any user facing changes?

Yes but backwards compatible
			pr-link: Alluxio/alluxio#18670
			change-id: cid-ae3e560225ca3f9d624bf321e75863f5834174b8
---
 .../src/main/java/alluxio/conf/PropertyKey.java   | 15 ++++++++-------
 .../alluxio/underfs/s3a/S3AUnderFileSystem.java   | 10 +++++-----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/core/common/src/main/java/alluxio/conf/PropertyKey.java b/core/common/src/main/java/alluxio/conf/PropertyKey.java
index 396f4d566fda..3bfd62bd63fd 100755
--- a/core/common/src/main/java/alluxio/conf/PropertyKey.java
+++ b/core/common/src/main/java/alluxio/conf/PropertyKey.java
@@ -1452,12 +1452,13 @@ public String toString() {
           .setConsistencyCheckLevel(ConsistencyCheckLevel.ENFORCE)
           .setScope(Scope.SERVER)
           .build();
-  public static final PropertyKey UNDERFS_S3_THREADS_MAX =
-      intBuilder(Name.UNDERFS_S3_THREADS_MAX)
-          .setDefaultValue(40)
-          .setDescription("The maximum number of threads to use for communicating with S3 and "
-              + "the maximum number of concurrent connections to S3. Includes both threads "
-              + "for data upload and metadata operations. This number should be at least as "
+  public static final PropertyKey UNDERFS_S3_CONNECTIONS_MAX =
+      intBuilder(Name.UNDERFS_S3_CONNECTIONS_MAX)
+          .setAlias("alluxio.underfs.s3.threads.max")
+          .setDefaultValue(1024)
+          .setDescription("The maximum number of concurrent connections to communicate with S3. "
+              + "This value includes both connections for data upload and metadata operations. "
+              + "This number should be at least as "
               + "large as the max admin threads plus max upload threads.")
           .setConsistencyCheckLevel(ConsistencyCheckLevel.WARN)
           .setScope(Scope.SERVER)
@@ -7952,7 +7953,7 @@ public static final class Name {
     public static final String UNDERFS_S3_PROXY_HOST = "alluxio.underfs.s3.proxy.host";
     public static final String UNDERFS_S3_PROXY_PORT = "alluxio.underfs.s3.proxy.port";
     public static final String UNDERFS_S3_REGION = "alluxio.underfs.s3.region";
-    public static final String UNDERFS_S3_THREADS_MAX = "alluxio.underfs.s3.threads.max";
+    public static final String UNDERFS_S3_CONNECTIONS_MAX = "alluxio.underfs.s3.connections.max";
     public static final String UNDERFS_S3_UPLOAD_THREADS_MAX =
         "alluxio.underfs.s3.upload.threads.max";
     public static final String KODO_ENDPOINT = "alluxio.underfs.kodo.endpoint";
diff --git a/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java b/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java
index 953d4369ff99..7ecd14d2f982 100644
--- a/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java
+++ b/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java
@@ -221,14 +221,14 @@ public static S3AUnderFileSystem createInstance(AlluxioURI uri,
     int numAdminThreads = conf.getInt(PropertyKey.UNDERFS_S3_ADMIN_THREADS_MAX);
     int numTransferThreads =
         conf.getInt(PropertyKey.UNDERFS_S3_UPLOAD_THREADS_MAX);
-    int numThreads = conf.getInt(PropertyKey.UNDERFS_S3_THREADS_MAX);
-    if (numThreads < numAdminThreads + numTransferThreads) {
+    int numConns = conf.getInt(PropertyKey.UNDERFS_S3_CONNECTIONS_MAX);
+    if (numConns < numAdminThreads + numTransferThreads) {
       LOG.warn("Configured s3 max threads ({}) is less than # admin threads ({}) plus transfer "
           + "threads ({}). Using admin threads + transfer threads as max threads instead.",
-              numThreads, numAdminThreads, numTransferThreads);
-      numThreads = numAdminThreads + numTransferThreads;
+              numConns, numAdminThreads, numTransferThreads);
+      numConns = numAdminThreads + numTransferThreads;
     }
-    clientConf.setMaxConnections(numThreads);
+    clientConf.setMaxConnections(numConns);
 
     // Set client request timeout for all requests since multipart copy is used,
     // and copy parts can only be set with the client configuration.