xiaoyuyao commented on a change in pull request #1163:
URL: https://github.com/apache/hadoop-ozone/pull/1163#discussion_r450526641
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java
##########
@@ -102,8 +102,15 @@ public ContainerReportHandler(final NodeManager
nodeManager,
public void onMessage(final ContainerReportFromDatanode reportFromDatanode,
final EventPublisher publisher) {
- final DatanodeDetails datanodeDetails =
+ final DatanodeDetails dnFromReport =
reportFromDatanode.getDatanodeDetails();
+ DatanodeDetails datanodeDetails =
Review comment:
When SCM does not have information about a datanode, it is usually the
datanode is dead. So skip processing those delayed report makes sense to me .
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -260,12 +260,21 @@ public RegisteredCommand register(
if (networkLocation != null) {
datanodeDetails.setNetworkLocation(networkLocation);
}
- nodeStateManager.addNode(datanodeDetails);
- clusterMap.add(datanodeDetails);
- addEntryTodnsToUuidMap(dnsName, datanodeDetails.getUuidString());
- // Updating Node Report, as registration is successful
- processNodeReport(datanodeDetails, nodeReport);
- LOG.info("Registered Data node : {}", datanodeDetails);
+ if (!isNodeRegistered(datanodeDetails)) {
+ clusterMap.add(datanodeDetails);
+ nodeStateManager.addNode(datanodeDetails);
+ // Check that datanode in nodeStateManager has topology parent set
+ DatanodeDetails dn =
Review comment:
Can we use nodeStateManager.getNode(datanodeDetails) to avoid the
UUID->String->UUID conversion?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -260,12 +260,21 @@ public RegisteredCommand register(
if (networkLocation != null) {
datanodeDetails.setNetworkLocation(networkLocation);
}
- nodeStateManager.addNode(datanodeDetails);
- clusterMap.add(datanodeDetails);
- addEntryTodnsToUuidMap(dnsName, datanodeDetails.getUuidString());
- // Updating Node Report, as registration is successful
- processNodeReport(datanodeDetails, nodeReport);
- LOG.info("Registered Data node : {}", datanodeDetails);
+ if (!isNodeRegistered(datanodeDetails)) {
Review comment:
Can we move isNodeRegistered check at the begin of register() to avoid
unnecessary dnsname lookup?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
