From 300d8ee4cdc6e947826d469442a41c5cfe0534b7 Mon Sep 17 00:00:00 2001 From: "zengqiang.xu" Date: Wed, 20 Jul 2022 20:34:30 +0800 Subject: [PATCH 1/5] HDFS-16671. RBF: RouterRpcFairnessPolicyController supports configurable permit acquire timeout --- ...ractRouterRpcFairnessPolicyController.java | 12 ++++++++- ...aticRouterRpcFairnessPolicyController.java | 11 ++++---- .../federation/router/RBFConfigKeys.java | 3 +++ .../src/main/resources/hdfs-rbf-default.xml | 8 ++++++ ...TestRouterRpcFairnessPolicyController.java | 25 +++++++++++++++++++ 5 files changed, 52 insertions(+), 7 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java index fe498c66b7ee8..9d5811f53bdb7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java @@ -29,6 +29,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY; + /** * Base fairness policy that implements @RouterRpcFairnessPolicyController. * Internally a map of nameservice to Semaphore is used to control permits. @@ -42,15 +45,22 @@ public class AbstractRouterRpcFairnessPolicyController /** Hash table to hold semaphore for each configured name service. */ private Map permits; + private long acquireTimeout = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; + public void init(Configuration conf) { this.permits = new HashMap<>(); + long timeout = conf.getLong(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY, + DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT); + if (timeout >= 0) { + acquireTimeout = timeout; + } } @Override public boolean acquirePermit(String nsId) { try { LOG.debug("Taking lock for nameservice {}", nsId); - return this.permits.get(nsId).tryAcquire(1, TimeUnit.SECONDS); + return this.permits.get(nsId).tryAcquire(acquireTimeout, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { LOG.debug("Cannot get a permit for nameservice {}", nsId); } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java index aa0777fc03d69..2a2a9192af7a7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java @@ -86,7 +86,7 @@ public void init(Configuration conf) // Assign remaining handlers equally to remaining name services and // general pool if applicable. if (!unassignedNS.isEmpty()) { - LOG.info("Unassigned ns {}", unassignedNS.toString()); + LOG.info("Unassigned ns {}", unassignedNS); int handlersPerNS = handlerCount / unassignedNS.size(); LOG.info("Handlers available per ns {}", handlersPerNS); for (String nsId : unassignedNS) { @@ -109,16 +109,15 @@ public void init(Configuration conf) } private static void logAssignment(String nsId, int count) { - LOG.info("Assigned {} handlers to nsId {} ", - count, nsId); + LOG.info("Assigned {} handlers to nsId {} ", count, nsId); } - private void validateHandlersCount(Configuration conf, int handlerCount, - Set allConfiguredNS) { + private void validateHandlersCount(Configuration conf, + int handlerCount, Set allConfiguredNS) { int totalDedicatedHandlers = 0; for (String nsId : allConfiguredNS) { int dedicatedHandlers = - conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); + conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); if (dedicatedHandlers > 0) { // Total handlers should not be less than sum of dedicated handlers. totalDedicatedHandlers += dedicatedHandlers; diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java index c0a9e3f294cd8..c6a8120bf2640 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java @@ -354,6 +354,9 @@ public class RBFConfigKeys extends CommonConfigurationKeysPublic { NoRouterRpcFairnessPolicyController.class; public static final String DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX = FEDERATION_ROUTER_FAIRNESS_PREFIX + "handler.count."; + public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY = + FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout"; + public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT = 1000; // HDFS Router Federation Rename. public static final String DFS_ROUTER_FEDERATION_RENAME_PREFIX = diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml index fcf6a28475fbd..08c3f142eaac0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml @@ -723,6 +723,14 @@ + + dfs.federation.router.fairness.acquire.timeout + 1000 + + The timeout value of acquiring permit for RouterRpcFairnessPolicyController. + + + dfs.federation.router.federation.rename.bandwidth 10 diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java index 8307f666b5d1c..be24dfe108b52 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java @@ -23,10 +23,12 @@ import org.apache.hadoop.hdfs.server.federation.router.FederationUtil; import org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys; import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.util.Time; import org.junit.Test; import org.slf4j.LoggerFactory; import static org.apache.hadoop.hdfs.server.federation.fairness.RouterRpcFairnessConstants.CONCURRENT_NS; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_HANDLER_COUNT_KEY; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_MONITOR_NAMENODE; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX; @@ -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_KEY, 100); + RouterRpcFairnessPolicyController routerRpcFairnessPolicyController = + FederationUtil.newFairnessPolicyController(conf); + + // ns1 should have 30 permits allocated + for (int i = 0; i < 30; i++) { + assertTrue(routerRpcFairnessPolicyController.acquirePermit("ns1")); + } + long acquireBeginTime = Time.monotonicNow(); + assertFalse(routerRpcFairnessPolicyController.acquirePermit("ns1")); + long acquireEndTime = Time.monotonicNow(); + + long acquireTime = acquireEndTime - acquireBeginTime; + + // There are some other operations, so acquireTime should more than 100ms. + assertTrue(acquireTime > 100); + assertTrue(acquireTime < 100 + 50); + } + @Test public void testAllocationErrorWithZeroHandlers() { Configuration conf = createConf(0); From bfb922926e46f644cad83389f6ca9c404085315d Mon Sep 17 00:00:00 2001 From: "zengqiang.xu" Date: Thu, 21 Jul 2022 11:01:33 +0800 Subject: [PATCH 2/5] HDFS-16671. Modify patch based on comments --- ...ractRouterRpcFairnessPolicyController.java | 24 +++++++++---------- ...aticRouterRpcFairnessPolicyController.java | 22 ++++++----------- .../federation/router/RBFConfigKeys.java | 7 +++--- .../src/main/resources/hdfs-rbf-default.xml | 4 ++-- ...TestRouterRpcFairnessPolicyController.java | 16 ++++++------- 5 files changed, 32 insertions(+), 41 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java index 9d5811f53bdb7..d83b6bf0b0088 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java @@ -30,7 +30,7 @@ import org.slf4j.LoggerFactory; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS; /** * Base fairness policy that implements @RouterRpcFairnessPolicyController. @@ -45,14 +45,14 @@ public class AbstractRouterRpcFairnessPolicyController /** Hash table to hold semaphore for each configured name service. */ private Map permits; - private long acquireTimeout = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; + private long acquireTimeoutMs = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; public void init(Configuration conf) { this.permits = new HashMap<>(); - long timeout = conf.getLong(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY, - DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT); - if (timeout >= 0) { - acquireTimeout = timeout; + long timeoutMs = conf.getTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS, + DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT, TimeUnit.MILLISECONDS); + if (timeoutMs >= 0) { + acquireTimeoutMs = timeoutMs; } } @@ -60,7 +60,7 @@ public void init(Configuration conf) { public boolean acquirePermit(String nsId) { try { LOG.debug("Taking lock for nameservice {}", nsId); - return this.permits.get(nsId).tryAcquire(acquireTimeout, TimeUnit.MILLISECONDS); + return this.permits.get(nsId).tryAcquire(acquireTimeoutMs, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { LOG.debug("Cannot get a permit for nameservice {}", nsId); } @@ -92,15 +92,13 @@ protected int getAvailablePermits(String nsId) { @Override public String getAvailableHandlerOnPerNs() { JSONObject json = new JSONObject(); - for (Map.Entry entry : permits.entrySet()) { + permits.forEach((k, v) -> { try { - String nsId = entry.getKey(); - int availableHandler = entry.getValue().availablePermits(); - json.put(nsId, availableHandler); + json.put(k, v.availablePermits()); } catch (JSONException e) { - LOG.warn("Cannot put {} into JSONObject", entry.getKey(), e); + LOG.warn("Cannot put {} into JSONObject", k, e); } - } + }); return json.toString(); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java index 2a2a9192af7a7..35045bdca8e27 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/StaticRouterRpcFairnessPolicyController.java @@ -50,13 +50,10 @@ public StaticRouterRpcFairnessPolicyController(Configuration conf) { init(conf); } - public void init(Configuration conf) - throws IllegalArgumentException { + public void init(Configuration conf) throws IllegalArgumentException { super.init(conf); // Total handlers configured to process all incoming Rpc. - int handlerCount = conf.getInt( - DFS_ROUTER_HANDLER_COUNT_KEY, - DFS_ROUTER_HANDLER_COUNT_DEFAULT); + int handlerCount = conf.getInt(DFS_ROUTER_HANDLER_COUNT_KEY, DFS_ROUTER_HANDLER_COUNT_DEFAULT); LOG.info("Handlers available for fairness assignment {} ", handlerCount); @@ -71,8 +68,7 @@ public void init(Configuration conf) allConfiguredNS.add(CONCURRENT_NS); validateHandlersCount(conf, handlerCount, allConfiguredNS); for (String nsId : allConfiguredNS) { - int dedicatedHandlers = - conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); + int dedicatedHandlers = conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); LOG.info("Dedicated handlers {} for ns {} ", dedicatedHandlers, nsId); if (dedicatedHandlers > 0) { handlerCount -= dedicatedHandlers; @@ -101,11 +97,9 @@ public void init(Configuration conf) int existingPermits = getAvailablePermits(CONCURRENT_NS); if (leftOverHandlers > 0) { LOG.info("Assigned extra {} handlers to commons pool", leftOverHandlers); - insertNameServiceWithPermits(CONCURRENT_NS, - existingPermits + leftOverHandlers); + insertNameServiceWithPermits(CONCURRENT_NS, existingPermits + leftOverHandlers); } - LOG.info("Final permit allocation for concurrent ns: {}", - getAvailablePermits(CONCURRENT_NS)); + LOG.info("Final permit allocation for concurrent ns: {}", getAvailablePermits(CONCURRENT_NS)); } private static void logAssignment(String nsId, int count) { @@ -116,8 +110,7 @@ private void validateHandlersCount(Configuration conf, int handlerCount, Set allConfiguredNS) { int totalDedicatedHandlers = 0; for (String nsId : allConfiguredNS) { - int dedicatedHandlers = - conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); + int dedicatedHandlers = conf.getInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + nsId, 0); if (dedicatedHandlers > 0) { // Total handlers should not be less than sum of dedicated handlers. totalDedicatedHandlers += dedicatedHandlers; @@ -127,8 +120,7 @@ private void validateHandlersCount(Configuration conf, } } if (totalDedicatedHandlers > handlerCount) { - String msg = String.format(ERROR_MSG, handlerCount, - totalDedicatedHandlers); + String msg = String.format(ERROR_MSG, handlerCount, totalDedicatedHandlers); LOG.error(msg); throw new IllegalArgumentException(msg); } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java index c6a8120bf2640..547b8132a385f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java @@ -354,9 +354,10 @@ public class RBFConfigKeys extends CommonConfigurationKeysPublic { NoRouterRpcFairnessPolicyController.class; public static final String DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX = FEDERATION_ROUTER_FAIRNESS_PREFIX + "handler.count."; - public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY = - FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout"; - public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT = 1000; + public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS = + FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout.ms"; + public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT = + TimeUnit.SECONDS.toMillis(1); // HDFS Router Federation Rename. public static final String DFS_ROUTER_FEDERATION_RENAME_PREFIX = diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml index 08c3f142eaac0..eb20491c33417 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml @@ -724,10 +724,10 @@ - dfs.federation.router.fairness.acquire.timeout + dfs.federation.router.fairness.acquire.timeout.ms 1000 - The timeout value of acquiring permit for RouterRpcFairnessPolicyController. + The maximum time, in milliseconds, to wait for a permit. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java index be24dfe108b52..7298e5eee7401 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java @@ -28,7 +28,7 @@ import org.slf4j.LoggerFactory; import static org.apache.hadoop.hdfs.server.federation.fairness.RouterRpcFairnessConstants.CONCURRENT_NS; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_KEY; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_HANDLER_COUNT_KEY; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_MONITOR_NAMENODE; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX; @@ -89,7 +89,7 @@ public void testHandlerAllocationPreconfigured() { 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_KEY, 100); + conf.setLong(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS, 100); RouterRpcFairnessPolicyController routerRpcFairnessPolicyController = FederationUtil.newFairnessPolicyController(conf); @@ -97,15 +97,15 @@ public void testAcquireTimeout() { for (int i = 0; i < 30; i++) { assertTrue(routerRpcFairnessPolicyController.acquirePermit("ns1")); } - long acquireBeginTime = Time.monotonicNow(); + long acquireBeginTimeMs = Time.monotonicNow(); assertFalse(routerRpcFairnessPolicyController.acquirePermit("ns1")); - long acquireEndTime = Time.monotonicNow(); + long acquireEndTimeMs = Time.monotonicNow(); - long acquireTime = acquireEndTime - acquireBeginTime; + long acquireTimeMs = acquireEndTimeMs - acquireBeginTimeMs; - // There are some other operations, so acquireTime should more than 100ms. - assertTrue(acquireTime > 100); - assertTrue(acquireTime < 100 + 50); + // There are some other operations, so acquireTimeMs >= 100ms. + assertTrue(acquireTimeMs >= 100); + assertTrue(acquireTimeMs < 100 + 50); } @Test From d985449246a38b8399ac9c716874924ef51da4bb Mon Sep 17 00:00:00 2001 From: "zengqiang.xu" Date: Fri, 22 Jul 2022 21:10:03 +0800 Subject: [PATCH 3/5] HDFS-16671. Modify patch based on comment. --- .../AbstractRouterRpcFairnessPolicyController.java | 10 +++++----- .../hdfs/server/federation/router/RBFConfigKeys.java | 6 +++--- .../src/main/resources/hdfs-rbf-default.xml | 6 +++--- .../TestRouterRpcFairnessPolicyController.java | 6 ++++-- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java index d83b6bf0b0088..d27153f7709e1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java @@ -29,8 +29,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT; /** * Base fairness policy that implements @RouterRpcFairnessPolicyController. @@ -45,12 +45,12 @@ public class AbstractRouterRpcFairnessPolicyController /** Hash table to hold semaphore for each configured name service. */ private Map permits; - private long acquireTimeoutMs = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; + private long acquireTimeoutMs = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT; public void init(Configuration conf) { this.permits = new HashMap<>(); - long timeoutMs = conf.getTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS, - DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT, TimeUnit.MILLISECONDS); + long timeoutMs = conf.getTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT, + DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT, TimeUnit.MILLISECONDS); if (timeoutMs >= 0) { acquireTimeoutMs = timeoutMs; } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java index 547b8132a385f..1a99deb9358b2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java @@ -354,9 +354,9 @@ public class RBFConfigKeys extends CommonConfigurationKeysPublic { NoRouterRpcFairnessPolicyController.class; public static final String DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX = FEDERATION_ROUTER_FAIRNESS_PREFIX + "handler.count."; - public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS = - FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout.ms"; - public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT = + public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT = + FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout"; + public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT = TimeUnit.SECONDS.toMillis(1); // HDFS Router Federation Rename. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml index eb20491c33417..51d9b8aabdd78 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/resources/hdfs-rbf-default.xml @@ -724,10 +724,10 @@ - dfs.federation.router.fairness.acquire.timeout.ms - 1000 + dfs.federation.router.fairness.acquire.timeout + 1s - The maximum time, in milliseconds, to wait for a permit. + The maximum time to wait for a permit. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java index 7298e5eee7401..10f80bb3c1266 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java @@ -27,8 +27,10 @@ import org.junit.Test; import org.slf4j.LoggerFactory; +import java.util.concurrent.TimeUnit; + import static org.apache.hadoop.hdfs.server.federation.fairness.RouterRpcFairnessConstants.CONCURRENT_NS; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_HANDLER_COUNT_KEY; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_MONITOR_NAMENODE; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX; @@ -89,7 +91,7 @@ public void testHandlerAllocationPreconfigured() { 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); + conf.setTimeDuration(DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT, 100, TimeUnit.MILLISECONDS); RouterRpcFairnessPolicyController routerRpcFairnessPolicyController = FederationUtil.newFairnessPolicyController(conf); From fcbdd2fbf85cffccef4ad55223d4f1236564450a Mon Sep 17 00:00:00 2001 From: "zengqiang.xu" Date: Sat, 23 Jul 2022 23:52:04 +0800 Subject: [PATCH 4/5] HDFS-16671. Modify code based comment --- .../AbstractRouterRpcFairnessPolicyController.java | 10 +++++++--- .../hdfs/server/federation/router/RBFConfigKeys.java | 2 +- .../TestRouterRpcFairnessPolicyController.java | 4 +--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java index d27153f7709e1..db917be71285c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/fairness/AbstractRouterRpcFairnessPolicyController.java @@ -30,7 +30,7 @@ import org.slf4j.LoggerFactory; import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT; -import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT; +import static org.apache.hadoop.hdfs.server.federation.router.RBFConfigKeys.DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT; /** * Base fairness policy that implements @RouterRpcFairnessPolicyController. @@ -45,14 +45,18 @@ public class AbstractRouterRpcFairnessPolicyController /** Hash table to hold semaphore for each configured name service. */ private Map permits; - private long acquireTimeoutMs = DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT; + 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, - DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT, TimeUnit.MILLISECONDS); + DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT, TimeUnit.MILLISECONDS); if (timeoutMs >= 0) { acquireTimeoutMs = timeoutMs; + } else { + LOG.warn("Invalid value {} configured for {} should be greater than or equal to 0. " + + "Using default value of : {}ms instead.", timeoutMs, + DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT, DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java index 1a99deb9358b2..3b6df418089e7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java @@ -356,7 +356,7 @@ public class RBFConfigKeys extends CommonConfigurationKeysPublic { FEDERATION_ROUTER_FAIRNESS_PREFIX + "handler.count."; public static final String DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT = FEDERATION_ROUTER_FAIRNESS_PREFIX + "acquire.timeout"; - public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_MS_DEFAULT = + public static final long DFS_ROUTER_FAIRNESS_ACQUIRE_TIMEOUT_DEFAULT = TimeUnit.SECONDS.toMillis(1); // HDFS Router Federation Rename. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java index 10f80bb3c1266..cd4ef2af8e761 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java @@ -101,9 +101,7 @@ public void testAcquireTimeout() { } long acquireBeginTimeMs = Time.monotonicNow(); assertFalse(routerRpcFairnessPolicyController.acquirePermit("ns1")); - long acquireEndTimeMs = Time.monotonicNow(); - - long acquireTimeMs = acquireEndTimeMs - acquireBeginTimeMs; + long acquireTimeMs = Time.monotonicNow() - acquireBeginTimeMs; // There are some other operations, so acquireTimeMs >= 100ms. assertTrue(acquireTimeMs >= 100); From e5436d5a832fb785127e8ab33519820efdb8b564 Mon Sep 17 00:00:00 2001 From: "zengqiang.xu" Date: Wed, 27 Jul 2022 23:02:35 +0800 Subject: [PATCH 5/5] HDFS-16671. Modify patch based on comments --- .../fairness/TestRouterRpcFairnessPolicyController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java index cd4ef2af8e761..1f5770b1ddaa3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterRpcFairnessPolicyController.java @@ -105,7 +105,6 @@ public void testAcquireTimeout() { // There are some other operations, so acquireTimeMs >= 100ms. assertTrue(acquireTimeMs >= 100); - assertTrue(acquireTimeMs < 100 + 50); } @Test