[ 
https://issues.apache.org/jira/browse/HDFS-13274?focusedWorklogId=792880&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-792880
 ]

ASF GitHub Bot logged work on HDFS-13274:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 19/Jul/22 18:58
            Start Date: 19/Jul/22 18:58
    Worklog Time Spent: 10m 
      Work Description: goiri commented on code in PR #4531:
URL: https://github.com/apache/hadoop/pull/4531#discussion_r924855528


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java:
##########
@@ -210,24 +217,22 @@ public AtomicInteger getClientIndex() {
    * @return Connection context.
    */
   protected ConnectionContext getConnection() {
-
     this.lastActiveTime = Time.now();
-
-    // Get a connection from the pool following round-robin
-    ConnectionContext conn = null;
     List<ConnectionContext> tmpConnections = this.connections;
-    int size = tmpConnections.size();
-    // Inc and mask off sign bit, lookup index should be non-negative int
-    int threadIndex = this.clientIndex.getAndIncrement() & 0x7FFFFFFF;
-    for (int i=0; i<size; i++) {
-      int index = (threadIndex + i) % size;
-      conn = tmpConnections.get(index);
-      if (conn != null && conn.isUsable()) {
-        return conn;
+    for (ConnectionContext tmpConnection : tmpConnections) {
+      if (tmpConnection != null && tmpConnection.isUsable()) {
+        return tmpConnection;
       }
     }
 
-    // We return a connection even if it's active
+    ConnectionContext conn = null;
+    // We return a connection even if it's busy
+    int size = tmpConnections.size();
+    if (size > 0) {
+      // Get a connection from the pool following round-robin
+      int threadIndex = this.clientIndex.getAndIncrement() & 0x7FFFFFFF;

Review Comment:
   We should keep the old comment:
   ```
   // Inc and mask off sign bit, lookup index should be non-negative int
   ```



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java:
##########
@@ -210,24 +217,22 @@ public AtomicInteger getClientIndex() {
    * @return Connection context.
    */
   protected ConnectionContext getConnection() {
-
     this.lastActiveTime = Time.now();
-
-    // Get a connection from the pool following round-robin
-    ConnectionContext conn = null;
     List<ConnectionContext> tmpConnections = this.connections;
-    int size = tmpConnections.size();
-    // Inc and mask off sign bit, lookup index should be non-negative int
-    int threadIndex = this.clientIndex.getAndIncrement() & 0x7FFFFFFF;
-    for (int i=0; i<size; i++) {
-      int index = (threadIndex + i) % size;
-      conn = tmpConnections.get(index);
-      if (conn != null && conn.isUsable()) {
-        return conn;
+    for (ConnectionContext tmpConnection : tmpConnections) {

Review Comment:
   Is there something more efficient we can do for this?
   Like sorting the list when we make the connection usable?
   It is not that expensive to do this but just in case.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml:
##########
@@ -134,6 +134,22 @@
     </description>
   </property>
 
+  <property>
+    <name>dfs.federation.router.enable.multiple.socket</name>
+    <value>false</value>
+    <description>
+      If enable multiple downstream socket or not.

Review Comment:
   
https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs-rbf/src/site/markdown/HDFSRouterFederation.md



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml:
##########
@@ -134,6 +134,22 @@
     </description>
   </property>
 
+  <property>
+    <name>dfs.federation.router.enable.multiple.socket</name>
+    <value>false</value>
+    <description>
+      If enable multiple downstream socket or not.

Review Comment:
   We should explain the relation between 
dfs.federation.router.enable.multiple.socket and 
dfs.federation.router.max.concurrency.per.connection.
   
   In general it would be good to have this in some of the RBF md files 
explaining why doing this.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 792880)
    Time Spent: 2h 40m  (was: 2.5h)

> RBF: Extend RouterRpcClient to use multiple sockets
> ---------------------------------------------------
>
>                 Key: HDFS-13274
>                 URL: https://issues.apache.org/jira/browse/HDFS-13274
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Íñigo Goiri
>            Assignee: Íñigo Goiri
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 2h 40m
>  Remaining Estimate: 0h
>
> HADOOP-13144 introduces the ability to create multiple connections for the 
> same user and use different sockets. The RouterRpcClient should use this 
> approach to get a better throughput.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to