You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/07/07 08:29:17 UTC
[GitHub] [ozone] symious opened a new pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
symious opened a new pull request #2392:
URL: https://github.com/apache/ozone/pull/2392
## What changes were proposed in this pull request?
Currently the "DatanodeDetails" are stored in Recon's local DB. Once a datanode has registered the content of DatanodeDetails won't be changed.
We met a scenario that datanodes' hostname is changed after running a while in Ozone cluster, the hostname in Recon's UI will still show the old hostnames.
This ticket proposes to send "reregisterCommand" to datanodes if the information of the datanode is changed.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5418
## How was this patch tested?
Unit test
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r672512695
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java
##########
@@ -173,6 +252,25 @@ protected void updateDatanodeOpState(DatanodeDetails reportedDn)
reportedDn.getPersistedOpStateExpiryEpochSec());
}
+ @Override
+ public RegisteredCommand register(
+ DatanodeDetails datanodeDetails, NodeReportProto nodeReport,
+ PipelineReportsProto pipelineReportsProto,
+ LayoutVersionProto layoutInfo) {
+ inMemDatanodeDetails.put(datanodeDetails.getUuid(), datanodeDetails);
+ if (isNodeRegistered(datanodeDetails)) {
+ try {
+ nodeDB.put(datanodeDetails.getUuid(), datanodeDetails);
Review comment:
Do we need the ReconNewNodeHandler after this change?
##########
File path: hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/datanodes/datanodes.tsx
##########
@@ -240,7 +240,7 @@ const COLUMNS = [
title: 'Version',
dataIndex: 'version',
key: 'version',
- isVisible: false,
Review comment:
Any reason why we have to always show thse mostly non changing data fields? (I believe they were left as isVisible = false and specific users can select them if needed)
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -139,18 +139,18 @@ public Response getDatanodes() {
}
DatanodeInfo dnInfo = (DatanodeInfo) datanode;
- datanodes.add(builder.withHostname(hostname)
+ datanodes.add(builder.withHostname(nodeManager.getHostName(datanode))
.withDatanodeStorageReport(storageReport)
.withLastHeartbeat(nodeManager.getLastHeartbeat(datanode))
.withState(nodeState)
.withOperationalState(nodeOpState)
.withPipelines(pipelines)
.withLeaderCount(leaderCount.get())
.withUUid(datanode.getUuidString())
- .withVersion(datanode.getVersion())
- .withSetupTime(datanode.getSetupTime())
- .withRevision(datanode.getRevision())
- .withBuildDate(datanode.getBuildDate())
+ .withVersion(nodeManager.getVersion(datanode))
Review comment:
I see that the in memory DN details are not seeded with the DB DN details on startup. This means that dead datanode information will never show up in Recon UI. While SCM is supposed to make use of only 'Live' nodes, Recon has to keep track of **all** nodes in the system.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r666485362
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java
##########
@@ -509,6 +510,14 @@ public int hashCode() {
return uuid.hashCode();
}
+ // Skip The OpStates which may change in Runtime.
+ public int getSignature() {
+ return Objects
+ .hash(uuid, uuidString, ipAddress, hostName, ports,
+ certSerialId, version, setupTime, revision, buildDate,
Review comment:
By including revision and build date here, are we aiming for re-registration every time the cluster is upgraded?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r667273774
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java
##########
@@ -509,6 +510,14 @@ public int hashCode() {
return uuid.hashCode();
}
+ // Skip The OpStates which may change in Runtime.
+ public int getSignature() {
+ return Objects
+ .hash(uuid, uuidString, ipAddress, hostName, ports,
+ certSerialId, version, setupTime, revision, buildDate,
Review comment:
I think it will be good to include this information here, we can show the updated detail information like version, build date on the UI to the users?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r672512695
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java
##########
@@ -173,6 +252,25 @@ protected void updateDatanodeOpState(DatanodeDetails reportedDn)
reportedDn.getPersistedOpStateExpiryEpochSec());
}
+ @Override
+ public RegisteredCommand register(
+ DatanodeDetails datanodeDetails, NodeReportProto nodeReport,
+ PipelineReportsProto pipelineReportsProto,
+ LayoutVersionProto layoutInfo) {
+ inMemDatanodeDetails.put(datanodeDetails.getUuid(), datanodeDetails);
+ if (isNodeRegistered(datanodeDetails)) {
+ try {
+ nodeDB.put(datanodeDetails.getUuid(), datanodeDetails);
Review comment:
Do we need the ReconNewNodeHandler after this change?
##########
File path: hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/datanodes/datanodes.tsx
##########
@@ -240,7 +240,7 @@ const COLUMNS = [
title: 'Version',
dataIndex: 'version',
key: 'version',
- isVisible: false,
Review comment:
Any reason why we have to always show thse mostly non changing data fields? (I believe they were left as isVisible = false and specific users can select them if needed)
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -139,18 +139,18 @@ public Response getDatanodes() {
}
DatanodeInfo dnInfo = (DatanodeInfo) datanode;
- datanodes.add(builder.withHostname(hostname)
+ datanodes.add(builder.withHostname(nodeManager.getHostName(datanode))
.withDatanodeStorageReport(storageReport)
.withLastHeartbeat(nodeManager.getLastHeartbeat(datanode))
.withState(nodeState)
.withOperationalState(nodeOpState)
.withPipelines(pipelines)
.withLeaderCount(leaderCount.get())
.withUUid(datanode.getUuidString())
- .withVersion(datanode.getVersion())
- .withSetupTime(datanode.getSetupTime())
- .withRevision(datanode.getRevision())
- .withBuildDate(datanode.getBuildDate())
+ .withVersion(nodeManager.getVersion(datanode))
Review comment:
I see that the in memory DN details are not seeded with the DB DN details on startup. This means that dead datanode information will never show up in Recon UI. While SCM is supposed to make use of only 'Live' nodes, Recon has to keep track of **all** nodes in the system.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r673625149
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java
##########
@@ -173,6 +252,25 @@ protected void updateDatanodeOpState(DatanodeDetails reportedDn)
reportedDn.getPersistedOpStateExpiryEpochSec());
}
+ @Override
+ public RegisteredCommand register(
+ DatanodeDetails datanodeDetails, NodeReportProto nodeReport,
+ PipelineReportsProto pipelineReportsProto,
+ LayoutVersionProto layoutInfo) {
+ inMemDatanodeDetails.put(datanodeDetails.getUuid(), datanodeDetails);
+ if (isNodeRegistered(datanodeDetails)) {
+ try {
+ nodeDB.put(datanodeDetails.getUuid(), datanodeDetails);
Review comment:
I think it's still needed. For new nodes, it will still need ReconNewNodeHandler to add to NodeDB.
For existing nodes, the `register` here is to keep nodeDB updated.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -139,18 +139,18 @@ public Response getDatanodes() {
}
DatanodeInfo dnInfo = (DatanodeInfo) datanode;
- datanodes.add(builder.withHostname(hostname)
+ datanodes.add(builder.withHostname(nodeManager.getHostName(datanode))
.withDatanodeStorageReport(storageReport)
.withLastHeartbeat(nodeManager.getLastHeartbeat(datanode))
.withState(nodeState)
.withOperationalState(nodeOpState)
.withPipelines(pipelines)
.withLeaderCount(leaderCount.get())
.withUUid(datanode.getUuidString())
- .withVersion(datanode.getVersion())
- .withSetupTime(datanode.getSetupTime())
- .withRevision(datanode.getRevision())
- .withBuildDate(datanode.getBuildDate())
+ .withVersion(nodeManager.getVersion(datanode))
Review comment:
I saw the original `loadingExistingDB` is calling the method of `register` for all existing datanodes, so it will be added to in-mem DB on startup.
##########
File path: hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/datanodes/datanodes.tsx
##########
@@ -240,7 +240,7 @@ const COLUMNS = [
title: 'Version',
dataIndex: 'version',
key: 'version',
- isVisible: false,
Review comment:
In fact, I didn't know these values can be selected from the UI, I was wondering if it's better to show the information to users directly.
I think the "setupTime" and "version" are quite good when the user is doing an upgrade or maintenance.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #2392: WIP: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#issuecomment-880168942
@symious Are you still working on more changes in this PR? I see the title as 'WIP'.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx merged pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2392:
URL: https://github.com/apache/ozone/pull/2392
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #2392: WIP: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#issuecomment-880566496
@avijayanhwx I did some changes on the commit to show more information like version, setup, etc.
Since the information is stored in `NodeStateMap` and doesn't support updates on this information, I think it's better to maintain an in-memory collection to store the updated datanodeDetails, besides I used the information from the in-memory collection to display on the Recon UI.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#issuecomment-876049346
@avijayanhwx Could you help to check this PR?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2392: HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2392:
URL: https://github.com/apache/ozone/pull/2392#discussion_r672512695
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java
##########
@@ -173,6 +252,25 @@ protected void updateDatanodeOpState(DatanodeDetails reportedDn)
reportedDn.getPersistedOpStateExpiryEpochSec());
}
+ @Override
+ public RegisteredCommand register(
+ DatanodeDetails datanodeDetails, NodeReportProto nodeReport,
+ PipelineReportsProto pipelineReportsProto,
+ LayoutVersionProto layoutInfo) {
+ inMemDatanodeDetails.put(datanodeDetails.getUuid(), datanodeDetails);
+ if (isNodeRegistered(datanodeDetails)) {
+ try {
+ nodeDB.put(datanodeDetails.getUuid(), datanodeDetails);
Review comment:
Do we need the ReconNewNodeHandler after this change?
##########
File path: hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/datanodes/datanodes.tsx
##########
@@ -240,7 +240,7 @@ const COLUMNS = [
title: 'Version',
dataIndex: 'version',
key: 'version',
- isVisible: false,
Review comment:
Any reason why we have to always show thse mostly non changing data fields? (I believe they were left as isVisible = false and specific users can select them if needed)
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -139,18 +139,18 @@ public Response getDatanodes() {
}
DatanodeInfo dnInfo = (DatanodeInfo) datanode;
- datanodes.add(builder.withHostname(hostname)
+ datanodes.add(builder.withHostname(nodeManager.getHostName(datanode))
.withDatanodeStorageReport(storageReport)
.withLastHeartbeat(nodeManager.getLastHeartbeat(datanode))
.withState(nodeState)
.withOperationalState(nodeOpState)
.withPipelines(pipelines)
.withLeaderCount(leaderCount.get())
.withUUid(datanode.getUuidString())
- .withVersion(datanode.getVersion())
- .withSetupTime(datanode.getSetupTime())
- .withRevision(datanode.getRevision())
- .withBuildDate(datanode.getBuildDate())
+ .withVersion(nodeManager.getVersion(datanode))
Review comment:
I see that the in memory DN details are not seeded with the DB DN details on startup. This means that dead datanode information will never show up in Recon UI. While SCM is supposed to make use of only 'Live' nodes, Recon has to keep track of **all** nodes in the system.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org