[
https://issues.apache.org/jira/browse/HADOOP-18915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17790691#comment-17790691
]
ASF GitHub Bot commented on HADOOP-18915:
-----------------------------------------
steveloughran commented on code in PR #6180:
URL: https://github.com/apache/hadoop/pull/6180#discussion_r1408127824
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/AWSClientConfig.java:
##########
@@ -371,24 +407,219 @@ private static void initSigner(Configuration conf,
}
/**
- * Configures request timeout.
+ * Configures request timeout in the client configuration.
+ * This is independent of the timeouts set in the sync and async HTTP
clients;
+ * the same method
*
* @param conf Hadoop configuration
* @param clientConfig AWS SDK configuration to update
*/
private static void initRequestTimeout(Configuration conf,
ClientOverrideConfiguration.Builder clientConfig) {
- long requestTimeoutMillis = conf.getTimeDuration(REQUEST_TIMEOUT,
- DEFAULT_REQUEST_TIMEOUT, TimeUnit.SECONDS, TimeUnit.MILLISECONDS);
+ // Get the connection settings
+ final ConnectionSettings conn = createApiConnectionSettings(conf);
+ final Duration callTimeout = conn.getApiCallTimeout();
- if (requestTimeoutMillis > Integer.MAX_VALUE) {
- LOG.debug("Request timeout is too high({} ms). Setting to {} ms instead",
- requestTimeoutMillis, Integer.MAX_VALUE);
- requestTimeoutMillis = Integer.MAX_VALUE;
+ if (callTimeout.toMillis() > 0) {
+ clientConfig.apiCallAttemptTimeout(callTimeout);
+ clientConfig.apiCallTimeout(callTimeout);
}
+ }
- if(requestTimeoutMillis > 0) {
-
clientConfig.apiCallAttemptTimeout(Duration.ofMillis(requestTimeoutMillis));
+ /**
+ * Reset the minimum operation duration to the default.
+ * Logs at INFO.
+ * <p>
+ * This MUST be called in test teardown in any test suite which
+ * called {@link #setMinimumOperationDuration(Duration)}.
+ */
+ public static void resetMinimumOperationDuration() {
+ setMinimumOperationDuration(MINIMUM_NETWORK_OPERATION_DURATION);
+ }
+
+ /**
+ * Set the minimum operation duration.
+ * This is for testing and will log at info; does require a non-negative
duration.
+ * <p>
+ * Test suites must call {@link #resetMinimumOperationDuration()} in their
teardown
+ * to avoid breaking subsequent tests in the same process.
+ * @param duration non-negative duration
+ * @throws IllegalArgumentException if the duration is negative.
+ */
+ @VisibleForTesting
+ public static void setMinimumOperationDuration(Duration duration) {
+ LOG.info("Setting minimum operation duration to {}ms",
duration.toMillis());
+ checkArgument(duration.compareTo(Duration.ZERO) >= 0,
+ "Duration must be positive: %sms", duration.toMillis());
+ minimumOperationDuration = duration;
+ }
+
+ /**
+ * Get the current minimum operation duration.
+ * @return current duration.
+ */
+ public static Duration getMinimumOperationDuration() {
+ return minimumOperationDuration;
+ }
+
+ /**
+ * All the connection settings, wrapped as a class for use by
+ * both the sync and async clients, and connection client builder.
+ */
+ static class ConnectionSettings {
+ private final int maxConnections;
+ private final boolean keepAlive;
+ private final Duration acquisitionTimeout;
+ private final Duration apiCallTimeout;
+ private final Duration connectionTTL;
+ private final Duration establishTimeout;
+ private final Duration maxIdleTime;
+ private final Duration socketTimeout;
+
+ ConnectionSettings(
+ final int maxConnections,
+ final boolean keepAlive,
+ final Duration apiCallTimeout,
+ final Duration acquisitionTimeout,
+ final Duration connectionTTL,
+ final Duration establishTimeout,
+ final Duration maxIdleTime,
+ final Duration socketTimeout) {
+ this.maxConnections = maxConnections;
+ this.keepAlive = keepAlive;
+ this.acquisitionTimeout = acquisitionTimeout;
+ this.apiCallTimeout = apiCallTimeout;
+ this.connectionTTL = connectionTTL;
+ this.establishTimeout = establishTimeout;
+ this.maxIdleTime = maxIdleTime;
+ this.socketTimeout = socketTimeout;
+ }
+
+ int getMaxConnections() {
+ return maxConnections;
+ }
+
+ boolean isKeepAlive() {
+ return keepAlive;
+ }
+
+ Duration getAcquisitionTimeout() {
+ return acquisitionTimeout;
+ }
+
+ Duration getApiCallTimeout() {
+ return apiCallTimeout;
+ }
+
+ Duration getConnectionTTL() {
+ return connectionTTL;
+ }
+
+ Duration getEstablishTimeout() {
+ return establishTimeout;
+ }
+
+ Duration getMaxIdleTime() {
+ return maxIdleTime;
+ }
+
+ Duration getSocketTimeout() {
+ return socketTimeout;
+ }
+
+ @Override
+ public String toString() {
+ return "ConnectionSettings{" +
+ "maxConnections=" + maxConnections +
+ ", keepAlive=" + keepAlive +
+ ", acquisitionTimeout=" + acquisitionTimeout +
+ ", apiCallTimeout=" + apiCallTimeout +
+ ", connectionTTL=" + connectionTTL +
+ ", establishTimeout=" + establishTimeout +
+ ", maxIdleTime=" + maxIdleTime +
+ ", socketTimeout=" + socketTimeout +
+ '}';
}
}
+
+
+ /**
+ * Build a connection settings object with only the settings used
+ * for the ClientConfig only.
+ * All other fields are null and MUST NOT be used.
+ * @param conf configuration to evaluate
+ * @return connection settings.
+ */
+ static ConnectionSettings createApiConnectionSettings(Configuration conf) {
Review Comment:
I've kept in in one place in case they ever need adding. I can add a new
ClientConnectionSettings struct there (which would be so much easier in java17)
> Extend S3A http client connection timeouts
> ------------------------------------------
>
> Key: HADOOP-18915
> URL: https://issues.apache.org/jira/browse/HADOOP-18915
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Ahmar Suhail
> Assignee: Steve Loughran
> Priority: Major
> Labels: pull-request-available
>
> * Add ability to configure *all* timeouts, especially acquisition time
> * recognise ApiCallTimeout and map tp a retryable exception
> * use getDuration so suffixes can be used -so remove all ambiguity about time
> unit
> * use units in core-default.xml so warnings aren't printed
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]