Skip to content

Commit

Permalink
Rename property key alluxio.underfs.s3.threads.max
Browse files Browse the repository at this point in the history
### 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: #18670
			change-id: cid-ae3e560225ca3f9d624bf321e75863f5834174b8
  • Loading branch information
apc999 authored Aug 8, 2024
1 parent df03c63 commit 9e373f0
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
15 changes: 8 additions & 7 deletions core/common/src/main/java/alluxio/conf/PropertyKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -1459,12 +1459,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)
Expand Down Expand Up @@ -8104,7 +8105,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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 9e373f0

Please sign in to comment.