[ 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