goiri commented on code in PR #4597:
URL: https://github.com/apache/hadoop/pull/4597#discussion_r927170619


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java:
##########
@@ -42,15 +45,22 @@
   /** Hash table to hold semaphore for each configured name service. */
   private Map<String, Semaphore> permits;
 
+  private long acquireTimeoutMs = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT;
+
   public void init(Configuration conf) {
     this.permits = new HashMap<>();
+    long timeoutMs = 
conf.getTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS,

Review Comment:
   If we do getTimeDuration() we don't need the prefix in the key 
DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS.
   We should set the default in the XML to "1s"



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java:
##########
@@ -83,6 +85,29 @@ public void testHandlerAllocationPreconfigured() {
     
assertFalse(routerRpcFairnessPolicyController.acquirePermit(CONCURRENT_NS));
   }
 
+  @Test
+  public void testAcquireTimeout() {
+    Configuration conf = createConf(40);
+    conf.setInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + "ns1", 30);
+    conf.setLong(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS, 100);

Review Comment:
   setTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT, 1, TimeUnit.SECONDS)



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml:
##########
@@ -723,6 +723,14 @@
     </description>
   </property>
 
+  <property>
+    <name>dfs.federation.router.fairness.acquire.timeout.ms</name>
+    <value>1000</value>

Review Comment:
   1s and remove the ms



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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