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

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

saxenapranav commented on code in PR #6633:
URL: https://github.com/apache/hadoop/pull/6633#discussion_r1546018428


##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheHttpClientFallback.java:
##########
@@ -0,0 +1,205 @@
+/**
+ * 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.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.ArrayList;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
+import org.apache.hadoop.fs.azurebfs.AbstractAbfsTestWithTimeout;
+import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
+import org.apache.hadoop.fs.azurebfs.constants.FSOperationType;
+import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
+import org.apache.hadoop.fs.azurebfs.utils.TracingHeaderFormat;
+
+import static java.net.HttpURLConnection.HTTP_OK;
+import static 
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APACHE_IMPL;
+import static 
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.JDK_FALLBACK;
+import static 
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.JDK_IMPL;
+import static 
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_APACHE_HTTP_CLIENT_MAX_IO_EXCEPTION_RETRIES;
+import static 
org.apache.hadoop.fs.azurebfs.services.HttpOperationType.APACHE_HTTP_CLIENT;
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+
+
+public class TestApacheHttpClientFallback extends AbstractAbfsTestWithTimeout {
+
+  public TestApacheHttpClientFallback() throws Exception {
+    super();
+  }
+
+  private TracingContext getSampleTracingContext() {
+    String correlationId, fsId;
+    TracingHeaderFormat format;
+    correlationId = "test-corr-id";
+    fsId = "test-filesystem-id";
+    format = TracingHeaderFormat.ALL_ID_FORMAT;
+    TracingContext tc = Mockito.spy(new TracingContext(correlationId, fsId,
+        FSOperationType.TEST_OP, true, format, null));
+    Mockito.doAnswer(answer -> {
+          answer.callRealMethod();
+          HttpOperation op = answer.getArgument(0);
+          if (op instanceof AbfsAHCHttpOperation) {
+            Assertions.assertThat(tc.getHeader()).endsWith(APACHE_IMPL);
+          }
+          if (op instanceof AbfsHttpOperation) {
+            if (ApacheHttpClientHealthMonitor.usable()) {
+              Assertions.assertThat(tc.getHeader()).endsWith(JDK_IMPL);
+            } else {
+              Assertions.assertThat(tc.getHeader()).endsWith(JDK_FALLBACK);
+            }
+          }
+          return null;
+        })
+        .when(tc)
+        .constructHeader(Mockito.any(HttpOperation.class),
+            Mockito.nullable(String.class), Mockito.nullable(String.class));
+    return tc;
+  }
+
+  @Test
+  public void testMultipleFailureLeadToFallback()
+      throws Exception {
+    TracingContext tc = getSampleTracingContext();
+    int[] retryIteration = {0};
+    intercept(IOException.class, () -> {
+      getMockRestOperation(retryIteration).execute(tc);
+    });
+    intercept(IOException.class, () -> {
+      getMockRestOperation(retryIteration).execute(tc);
+    });
+  }
+
+  private AbfsRestOperation getMockRestOperation(int[] retryIteration)
+      throws IOException {
+    AbfsConfiguration configuration = Mockito.mock(AbfsConfiguration.class);
+    Mockito.doReturn(APACHE_HTTP_CLIENT)
+        .when(configuration)
+        .getPreferredHttpOperationType();
+    Mockito.doReturn(DEFAULT_APACHE_HTTP_CLIENT_MAX_IO_EXCEPTION_RETRIES)
+        .when(configuration)
+        .getMaxApacheHttpClientIoExceptions();
+    AbfsClient client = Mockito.mock(AbfsClient.class);
+    Mockito.doReturn(Mockito.mock(ExponentialRetryPolicy.class))
+        .when(client)
+        .getExponentialRetryPolicy();
+
+    AbfsRetryPolicy retryPolicy = Mockito.mock(AbfsRetryPolicy.class);
+    Mockito.doReturn(retryPolicy)
+        .when(client)
+        .getRetryPolicy(Mockito.nullable(String.class));
+
+    Mockito.doAnswer(answer -> {
+          if (retryIteration[0]
+              < DEFAULT_APACHE_HTTP_CLIENT_MAX_IO_EXCEPTION_RETRIES) {
+            retryIteration[0]++;
+            return true;
+          } else {
+            return false;
+          }
+        })
+        .when(retryPolicy)
+        .shouldRetry(Mockito.anyInt(), Mockito.nullable(Integer.class));
+
+    AbfsThrottlingIntercept abfsThrottlingIntercept = Mockito.mock(
+        AbfsThrottlingIntercept.class);
+    Mockito.doNothing()
+        .when(abfsThrottlingIntercept)
+        .updateMetrics(Mockito.any(AbfsRestOperationType.class),
+            Mockito.any(HttpOperation.class));
+    Mockito.doNothing()
+        .when(abfsThrottlingIntercept)
+        .sendingRequest(Mockito.any(AbfsRestOperationType.class),
+            Mockito.nullable(AbfsCounters.class));
+    Mockito.doReturn(abfsThrottlingIntercept).when(client).getIntercept();
+
+
+    AbfsRestOperation op = Mockito.spy(new AbfsRestOperation(
+        AbfsRestOperationType.ReadFile,
+        client,
+        AbfsHttpConstants.HTTP_METHOD_GET,
+        new URL("http://localhost";),
+        new ArrayList<>(),
+        null,
+        configuration,
+        "clientId"
+    ));
+
+    Mockito.doReturn(null).when(op).getClientLatency();
+
+    Mockito.doReturn(createApacheHttpOp())
+        .when(op)
+        .createAbfsHttpOperation();
+    Mockito.doReturn(createAhcHttpOp())
+        .when(op)
+        .createAbfsAHCHttpOperation();
+
+    Mockito.doAnswer(answer -> {
+      return answer.getArgument(0);
+    
}).when(op).createNewTracingContext(Mockito.nullable(TracingContext.class));
+
+    Mockito.doNothing()
+        .when(op)
+        .signRequest(Mockito.any(HttpOperation.class), Mockito.anyInt());
+
+    Mockito.doAnswer(answer -> {
+      HttpOperation operation = Mockito.spy(
+          (HttpOperation) answer.callRealMethod());
+      Assertions.assertThat(operation).isInstanceOf(
+          retryIteration[0]
+              < DEFAULT_APACHE_HTTP_CLIENT_MAX_IO_EXCEPTION_RETRIES
+              ? AbfsAHCHttpOperation.class
+              : AbfsHttpOperation.class);
+      Mockito.doReturn(HTTP_OK).when(operation).getStatusCode();
+      Mockito.doThrow(new IOException("Test Exception"))
+          .when(operation)
+          .processResponse(Mockito.nullable(byte[].class), Mockito.anyInt(),
+              Mockito.anyInt());
+      Mockito.doCallRealMethod().when(operation).getTracingContextSuffix();
+      return operation;
+    }).when(op).createHttpOperation();
+    return op;
+  }
+
+  private AbfsAHCHttpOperation createAhcHttpOp() {
+    AbfsAHCHttpOperation ahcOp = Mockito.mock(AbfsAHCHttpOperation.class);
+    Mockito.doCallRealMethod().when(ahcOp).getTracingContextSuffix();
+    return ahcOp;
+  }
+
+  private AbfsHttpOperation createApacheHttpOp() {
+    AbfsHttpOperation httpOperationMock = 
Mockito.mock(AbfsHttpOperation.class);
+    Mockito.doCallRealMethod()
+        .when(httpOperationMock)
+        .getTracingContextSuffix();
+    return httpOperationMock;
+  }
+
+  @Test
+  public void testTcHeaderOnJDKClientUse() {
+    TracingContext tc = getSampleTracingContext();
+    AbfsHttpOperation op = Mockito.mock(AbfsHttpOperation.class);
+    Mockito.doCallRealMethod().when(op).getTracingContextSuffix();
+    tc.constructHeader(op, null, null);
+  }

Review Comment:
   There was only one request that was made after fallback. Have increased the 
number of request post fallback and asserting in them if they are JDK_FALLBACK.





> [ABFS]: ApacheHttpClient adaptation as network library
> ------------------------------------------------------
>
>                 Key: HADOOP-19120
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19120
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/azure
>    Affects Versions: 3.5.0
>            Reporter: Pranav Saxena
>            Assignee: Pranav Saxena
>            Priority: Major
>              Labels: pull-request-available
>
> Apache HttpClient is more feature-rich and flexible and gives application 
> more granular control over networking parameter.
> ABFS currently relies on the JDK-net library. This library is managed by 
> OpenJDK and has no performance problem. However, it limits the application's 
> control over networking, and there are very few APIs and hooks exposed that 
> the application can use to get metrics, choose which and when a connection 
> should be reused. ApacheHttpClient will give important hooks to fetch 
> important metrics and control networking parameters.
> A custom implementation of connection-pool is used. The implementation is 
> adapted from the JDK8 connection pooling. Reasons for doing it:
> 1. PoolingHttpClientConnectionManager heuristic caches all the reusable 
> connections it has created. JDK's implementation only caches limited number 
> of connections. The limit is given by JVM system property 
> "http.maxConnections". If there is no system-property, it defaults to 5. 
> Connection-establishment latency increased with all the connections were 
> cached. Hence, adapting the pooling heuristic of JDK netlib,
> 2. In PoolingHttpClientConnectionManager, it expects the application to 
> provide `setMaxPerRoute` and `setMaxTotal`, which the implementation uses as 
> the total number of connections it can create. For application using ABFS, it 
> is not feasible to provide a value in the initialisation of the 
> connectionManager. JDK's implementation has no cap on the number of 
> connections it can have opened on a moment. Hence, adapting the pooling 
> heuristic of JDK netlib,



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