[ 
https://issues.apache.org/jira/browse/HADOOP-18915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17790694#comment-17790694
 ] 

ASF GitHub Bot commented on HADOOP-18915:
-----------------------------------------

steveloughran commented on code in PR #6180:
URL: https://github.com/apache/hadoop/pull/6180#discussion_r1408129802


##########
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 {

Review Comment:
   no, its a static struct and a builder is simply a java-lang workaround for 
the lack of named parameters like scala and python or default arguments like 
C++. this is just a 7-tuple





> 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: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to