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

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

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


##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java:
##########
@@ -76,14 +77,46 @@ public void testPoolWithZeroSysProp() throws Exception {
   @Test
   public void testEmptySizePool() throws Exception {
     Configuration configuration = new Configuration();
-    configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE, 
"0");
-    AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, 
EMPTY_STRING);
-    try (KeepAliveCache keepAliveCache = new 
KeepAliveCache(abfsConfiguration)) {
-      
Assertions.assertThat(keepAliveCache.put(Mockito.mock(HttpClientConnection.class))).isFalse();
-      Assertions.assertThat(keepAliveCache.get()).isNull();
+    configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE,
+        "0");
+    AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration,
+        EMPTY_STRING);
+    try (KeepAliveCache keepAliveCache = new KeepAliveCache(
+        abfsConfiguration)) {
+      assertCachePutFail(keepAliveCache,
+          Mockito.mock(HttpClientConnection.class));
+      assertCacheGetNull(keepAliveCache);
     }
   }
 
+  private void assertCacheGetNull(final KeepAliveCache keepAliveCache)

Review Comment:
   Makes sense. Have rename to `assertCacheGetIsNul`l and 
`assertCacheGetIsNonNull`



##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java:
##########
@@ -76,14 +77,46 @@ public void testPoolWithZeroSysProp() throws Exception {
   @Test
   public void testEmptySizePool() throws Exception {
     Configuration configuration = new Configuration();
-    configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE, 
"0");
-    AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, 
EMPTY_STRING);
-    try (KeepAliveCache keepAliveCache = new 
KeepAliveCache(abfsConfiguration)) {
-      
Assertions.assertThat(keepAliveCache.put(Mockito.mock(HttpClientConnection.class))).isFalse();
-      Assertions.assertThat(keepAliveCache.get()).isNull();
+    configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE,
+        "0");
+    AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration,
+        EMPTY_STRING);
+    try (KeepAliveCache keepAliveCache = new KeepAliveCache(
+        abfsConfiguration)) {
+      assertCachePutFail(keepAliveCache,
+          Mockito.mock(HttpClientConnection.class));
+      assertCacheGetNull(keepAliveCache);
     }
   }
 
+  private void assertCacheGetNull(final KeepAliveCache keepAliveCache)

Review Comment:
   Makes sense. Have renamed to `assertCacheGetIsNul`l and 
`assertCacheGetIsNonNull`





> [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
>             Fix For: 3.5.0, 3.4.1
>
>
> 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: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to