From 313e8b9f1344da376de2b0e899e78e384f37ff35 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 5 Mar 2019 11:46:36 -0800 Subject: [PATCH] HDDS-1193. Refactor ContainerChillModeRule and DatanodeChillMode rule. (#534) * HDDS-1193. Refactor ContainerChillModeRule and DatanodeChillMode rule. --- .../scm/chillmode/ContainerChillModeRule.java | 43 +++++++++++++------ .../scm/chillmode/DataNodeChillModeRule.java | 41 +++++++++++++----- .../scm/chillmode/SCMChillModeManager.java | 30 +++++-------- .../scm/server/StorageContainerManager.java | 2 - .../chillmode/TestSCMChillModeManager.java | 14 ++---- 5 files changed, 75 insertions(+), 55 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/ContainerChillModeRule.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/ContainerChillModeRule.java index 95785323e3..17dd496915 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/ContainerChillModeRule.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/ContainerChillModeRule.java @@ -29,12 +29,15 @@ import org.apache.hadoop.hdds.scm.server.SCMDatanodeProtocolServer.NodeRegistrationContainerReport; import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.hdds.server.events.EventHandler; +import org.apache.hadoop.hdds.server.events.EventPublisher; /** * Class defining Chill mode exit criteria for Containers. */ public class ContainerChillModeRule implements - ChillModeExitRule { + ChillModeExitRule, + EventHandler { // Required cutoff % for containers with at least 1 reported replica. private double chillModeCutoff; @@ -68,9 +71,6 @@ public ContainerChillModeRule(Configuration conf, @Override public boolean validate() { - if (maxContainer == 0) { - return true; - } return getCurrentContainerThreshold() >= chillModeCutoff; } @@ -84,10 +84,6 @@ public double getCurrentContainerThreshold() { @Override public void process(NodeRegistrationContainerReport reportsProto) { - if (maxContainer == 0) { - // No container to check. - return; - } reportsProto.getReport().getReportsList().forEach(c -> { if (containerMap.containsKey(c.getContainerID())) { @@ -96,12 +92,33 @@ public void process(NodeRegistrationContainerReport reportsProto) { } } }); - if(chillModeManager.getInChillMode()) { - SCMChillModeManager.getLogger().info( - "SCM in chill mode. {} % containers have at least one" - + " reported replica.", - (containerWithMinReplicas.get() / maxContainer) * 100); + } + + @Override + public void onMessage(NodeRegistrationContainerReport + nodeRegistrationContainerReport, EventPublisher publisher) { + + // TODO: when we have remove handlers, we can remove getInChillmode check + + if (chillModeManager.getInChillMode()) { + if (validate()) { + return; + } + + process(nodeRegistrationContainerReport); + if (chillModeManager.getInChillMode()) { + SCMChillModeManager.getLogger().info( + "SCM in chill mode. {} % containers have at least one" + + " reported replica.", + (containerWithMinReplicas.get() / maxContainer) * 100); + } + + if (validate()) { + chillModeManager.validateChillModeExitRules(publisher); + } + } + } @Override diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/DataNodeChillModeRule.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/DataNodeChillModeRule.java index 39505056c8..be99962908 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/DataNodeChillModeRule.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/DataNodeChillModeRule.java @@ -25,13 +25,16 @@ import org.apache.hadoop.hdds.scm.server.SCMDatanodeProtocolServer.NodeRegistrationContainerReport; import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.hdds.server.events.EventHandler; +import org.apache.hadoop.hdds.server.events.EventPublisher; /** * Class defining Chill mode exit criteria according to number of DataNodes * registered with SCM. */ public class DataNodeChillModeRule implements - ChillModeExitRule { + ChillModeExitRule, + EventHandler { // Min DataNodes required to exit chill mode. private int requiredDns; @@ -62,17 +65,33 @@ public double getRegisteredDataNodes() { @Override public void process(NodeRegistrationContainerReport reportsProto) { - if (requiredDns == 0) { - // No dn check required. - return; - } - if(chillModeManager.getInChillMode()) { - registeredDnSet.add(reportsProto.getDatanodeDetails().getUuid()); - registeredDns = registeredDnSet.size(); - SCMChillModeManager.getLogger().info( - "SCM in chill mode. {} DataNodes registered, {} required.", - registeredDns, requiredDns); + registeredDnSet.add(reportsProto.getDatanodeDetails().getUuid()); + registeredDns = registeredDnSet.size(); + + } + + @Override + public void onMessage(NodeRegistrationContainerReport + nodeRegistrationContainerReport, EventPublisher publisher) { + // TODO: when we have remove handlers, we can remove getInChillmode check + + if (chillModeManager.getInChillMode()) { + if (validate()) { + return; + } + + process(nodeRegistrationContainerReport); + + if (chillModeManager.getInChillMode()) { + SCMChillModeManager.getLogger().info( + "SCM in chill mode. {} DataNodes registered, {} required.", + registeredDns, requiredDns); + } + + if (validate()) { + chillModeManager.validateChillModeExitRules(publisher); + } } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/SCMChillModeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/SCMChillModeManager.java index 1d97a579c8..8ced6d5a95 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/SCMChillModeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/chillmode/SCMChillModeManager.java @@ -28,9 +28,6 @@ import org.apache.hadoop.hdds.scm.events.SCMEvents; import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; import org.apache.hadoop.hdds.scm.pipeline.RatisPipelineUtils; -import org.apache.hadoop.hdds.scm.server.SCMDatanodeProtocolServer - .NodeRegistrationContainerReport; -import org.apache.hadoop.hdds.server.events.EventHandler; import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.hdds.server.events.EventQueue; import org.slf4j.Logger; @@ -48,8 +45,7 @@ * for reported containers and validates if cutoff threshold for * containers is meet. */ -public class SCMChillModeManager implements - EventHandler { +public class SCMChillModeManager { private static final Logger LOG = LoggerFactory.getLogger(SCMChillModeManager.class); @@ -78,9 +74,16 @@ public SCMChillModeManager(Configuration conf, HddsConfigKeys.HDDS_SCM_CHILLMODE_ENABLED, HddsConfigKeys.HDDS_SCM_CHILLMODE_ENABLED_DEFAULT); if (isChillModeEnabled) { - exitRules.put(CONT_EXIT_RULE, - new ContainerChillModeRule(config, allContainers, this)); - exitRules.put(DN_EXIT_RULE, new DataNodeChillModeRule(config, this)); + ContainerChillModeRule containerChillModeRule = + new ContainerChillModeRule(config, allContainers, this); + DataNodeChillModeRule dataNodeChillModeRule = + new DataNodeChillModeRule(config, this); + exitRules.put(CONT_EXIT_RULE, containerChillModeRule); + exitRules.put(DN_EXIT_RULE, dataNodeChillModeRule); + eventPublisher.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, + containerChillModeRule); + eventPublisher.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, + dataNodeChillModeRule); if (conf.getBoolean( HddsConfigKeys.HDDS_SCM_CHILLMODE_PIPELINE_AVAILABILITY_CHECK, @@ -146,17 +149,6 @@ public void exitChillMode(EventPublisher eventQueue) { .scheduleFixedIntervalPipelineCreator(pipelineManager, config); } - @Override - public void onMessage( - NodeRegistrationContainerReport nodeRegistrationContainerReport, - EventPublisher publisher) { - if (getInChillMode()) { - exitRules.get(CONT_EXIT_RULE).process(nodeRegistrationContainerReport); - exitRules.get(DN_EXIT_RULE).process(nodeRegistrationContainerReport); - validateChillModeExitRules(publisher); - } - } - public boolean getInChillMode() { if (!isChillModeEnabled) { return false; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index fc93235b7b..c9446a506a 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -364,8 +364,6 @@ public StorageContainerManager(OzoneConfiguration conf, replicationStatus.getChillModeStatusListener()); eventQueue.addHandler(SCMEvents.CHILL_MODE_STATUS, (BlockManagerImpl) scmBlockManager); - eventQueue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); registerMXBean(); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/chillmode/TestSCMChillModeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/chillmode/TestSCMChillModeManager.java index faf8fee8e6..957fe70824 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/chillmode/TestSCMChillModeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/chillmode/TestSCMChillModeManager.java @@ -90,8 +90,7 @@ private void testChillMode(int numContainers) throws Exception { } scmChillModeManager = new SCMChillModeManager( config, containers, null, queue); - queue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); + assertTrue(scmChillModeManager.getInChillMode()); queue.fireEvent(SCMEvents.NODE_REGISTRATION_CONT_REPORT, HddsTestUtils.createNodeRegistrationContainerReport(containers)); @@ -111,8 +110,7 @@ public void testChillModeExitRule() throws Exception { } scmChillModeManager = new SCMChillModeManager( config, containers, null, queue); - queue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); + assertTrue(scmChillModeManager.getInChillMode()); testContainerThreshold(containers.subList(0, 25), 0.25); @@ -167,8 +165,7 @@ public void testContainerChillModeRule() throws Exception { scmChillModeManager = new SCMChillModeManager( config, containers, null, queue); - queue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); + assertTrue(scmChillModeManager.getInChillMode()); // When 10 CLOSED containers are reported by DNs, the computed container @@ -192,8 +189,7 @@ private void testChillModeDataNodes(int numOfDns) throws Exception { conf.setInt(HddsConfigKeys.HDDS_SCM_CHILLMODE_MIN_DATANODE, numOfDns); scmChillModeManager = new SCMChillModeManager( conf, containers, null, queue); - queue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); + // Assert SCM is in Chill mode. assertTrue(scmChillModeManager.getInChillMode()); @@ -256,8 +252,6 @@ public void testChillModePipelineExitRule() throws Exception { scmChillModeManager = new SCMChillModeManager( config, containers, pipelineManager, queue); - queue.addHandler(SCMEvents.NODE_REGISTRATION_CONT_REPORT, - scmChillModeManager); queue.fireEvent(SCMEvents.NODE_REGISTRATION_CONT_REPORT, HddsTestUtils.createNodeRegistrationContainerReport(containers));