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 49a9075829..b4e3dc36b0 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 @@ -42,6 +42,10 @@ public class StaticRouterRpcFairnessPolicyController extends private static final Logger LOG = LoggerFactory.getLogger(StaticRouterRpcFairnessPolicyController.class); + public static final String ERROR_MSG = "Configured handlers " + + DFS_ROUTER_HANDLER_COUNT_KEY + '=' + + " %d is less than the minimum required handlers %d"; + public StaticRouterRpcFairnessPolicyController(Configuration conf) { init(conf); } @@ -65,15 +69,13 @@ public void init(Configuration conf) // Insert the concurrent nameservice into the set to process together 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); LOG.info("Dedicated handlers {} for ns {} ", dedicatedHandlers, nsId); if (dedicatedHandlers > 0) { handlerCount -= dedicatedHandlers; - // Total handlers should not be less than sum of dedicated - // handlers. - validateCount(nsId, handlerCount, 0); insertNameServiceWithPermits(nsId, dedicatedHandlers); logAssignment(nsId, dedicatedHandlers); } else { @@ -88,8 +90,6 @@ public void init(Configuration conf) int handlersPerNS = handlerCount / unassignedNS.size(); LOG.info("Handlers available per ns {}", handlersPerNS); for (String nsId : unassignedNS) { - // Each NS should have at least one handler assigned. - validateCount(nsId, handlersPerNS, 1); insertNameServiceWithPermits(nsId, handlersPerNS); logAssignment(nsId, handlersPerNS); } @@ -112,15 +112,26 @@ private static void logAssignment(String nsId, int count) { count, nsId); } - private static void validateCount(String nsId, int handlers, int min) throws - IllegalArgumentException { - if (handlers < min) { - String msg = - "Available handlers " + handlers + - " lower than min " + min + - " for nsId " + nsId; + 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); + if (dedicatedHandlers > 0) { + // Total handlers should not be less than sum of dedicated handlers. + totalDedicatedHandlers += dedicatedHandlers; + } else { + // Each NS should have at least one handler assigned. + totalDedicatedHandlers++; + } + } + if (totalDedicatedHandlers > handlerCount) { + 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/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 c0c30747d8..8e816643ac 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 @@ -85,30 +85,31 @@ public void testHandlerAllocationPreconfigured() { @Test public void testAllocationErrorWithZeroHandlers() { Configuration conf = createConf(0); - verifyInstantiationError(conf); + verifyInstantiationError(conf, 0, 3); } @Test public void testAllocationErrorForLowDefaultHandlers() { Configuration conf = createConf(1); - verifyInstantiationError(conf); + verifyInstantiationError(conf, 1, 3); } @Test public void testAllocationErrorForLowDefaultHandlersPerNS() { Configuration conf = createConf(1); conf.setInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + "concurrent", 1); - verifyInstantiationError(conf); + verifyInstantiationError(conf, 1, 3); } @Test public void testAllocationErrorForLowPreconfiguredHandlers() { Configuration conf = createConf(1); conf.setInt(DFS_ROUTER_FAIR_HANDLER_COUNT_KEY_PREFIX + "ns1", 2); - verifyInstantiationError(conf); + verifyInstantiationError(conf, 1, 4); } - private void verifyInstantiationError(Configuration conf) { + private void verifyInstantiationError(Configuration conf, int handlerCount, + int totalDedicatedHandlers) { GenericTestUtils.LogCapturer logs = GenericTestUtils.LogCapturer .captureLogs(LoggerFactory.getLogger( StaticRouterRpcFairnessPolicyController.class)); @@ -117,8 +118,11 @@ private void verifyInstantiationError(Configuration conf) { } catch (IllegalArgumentException e) { // Ignore the exception as it is expected here. } - assertTrue("Should contain error message", - logs.getOutput().contains("lower than min")); + String errorMsg = String.format( + StaticRouterRpcFairnessPolicyController.ERROR_MSG, handlerCount, + totalDedicatedHandlers); + assertTrue("Should contain error message: " + errorMsg, + logs.getOutput().contains(errorMsg)); } private RouterRpcFairnessPolicyController getFairnessPolicyController(