siddhantsangwan commented on code in PR #7155:
URL: https://github.com/apache/ozone/pull/7155#discussion_r1745951180
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -391,17 +391,21 @@ private synchronized boolean
checkIfDecommissionPossible(List<DatanodeDetails> d
int numDecom = dns.size();
List<DatanodeDetails> validDns = new ArrayList<>(dns);
int inServiceTotal =
nodeManager.getNodeCount(NodeStatus.inServiceHealthy());
+ int unHealthyTotal = nodeManager.getNodeCount(NodeStatus.inServiceStale())
+
+ nodeManager.getNodeCount(NodeStatus.inServiceDead());
Review Comment:
I think what we really need is the total of all nodes that are not
(in-service and healthy). That is, `totalNodes - inServiceTotal`. This is
because nodes that are entering maintenance, in maintenance, decommissioning
etc. also don't count toward in-service healthy nodes.
The easiest way I can see to get `totalNodes` is to use the following API
```
List<DatanodeDetails> getAllNodes();
```
and then get the size.
`NodeStateManager` has a `getTotalNodeCount` API but unfortunately it's not
exposed in the interface. We can use `getAllNodes` for now. And later, in
another jira, we can expose `getTotalNodeCount` in the interface and use it.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -391,17 +391,21 @@ private synchronized boolean
checkIfDecommissionPossible(List<DatanodeDetails> d
int numDecom = dns.size();
List<DatanodeDetails> validDns = new ArrayList<>(dns);
int inServiceTotal =
nodeManager.getNodeCount(NodeStatus.inServiceHealthy());
+ int unHealthyTotal = nodeManager.getNodeCount(NodeStatus.inServiceStale())
+
+ nodeManager.getNodeCount(NodeStatus.inServiceDead());
for (DatanodeDetails dn : dns) {
try {
NodeStatus nodeStatus = getNodeStatus(dn);
NodeOperationalState opState = nodeStatus.getOperationalState();
if (opState != NodeOperationalState.IN_SERVICE) {
numDecom--;
validDns.remove(dn);
+ LOG.warn("Cannot decommission " + dn.getHostName() + " because it is
not IN-SERVICE");
Review Comment:
IntelliJ's suggestion:
```suggestion
LOG.warn("Cannot decommission {} because it is not IN-SERVICE",
dn.getHostName());
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -391,17 +391,21 @@ private synchronized boolean
checkIfDecommissionPossible(List<DatanodeDetails> d
int numDecom = dns.size();
List<DatanodeDetails> validDns = new ArrayList<>(dns);
int inServiceTotal =
nodeManager.getNodeCount(NodeStatus.inServiceHealthy());
+ int unHealthyTotal = nodeManager.getNodeCount(NodeStatus.inServiceStale())
+
+ nodeManager.getNodeCount(NodeStatus.inServiceDead());
Review Comment:
Let's move this computation to the `if` block where the error message is
being created. So we perform this computation only if needed.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -391,17 +391,21 @@ private synchronized boolean
checkIfDecommissionPossible(List<DatanodeDetails> d
int numDecom = dns.size();
List<DatanodeDetails> validDns = new ArrayList<>(dns);
int inServiceTotal =
nodeManager.getNodeCount(NodeStatus.inServiceHealthy());
+ int unHealthyTotal = nodeManager.getNodeCount(NodeStatus.inServiceStale())
+
+ nodeManager.getNodeCount(NodeStatus.inServiceDead());
for (DatanodeDetails dn : dns) {
try {
NodeStatus nodeStatus = getNodeStatus(dn);
NodeOperationalState opState = nodeStatus.getOperationalState();
if (opState != NodeOperationalState.IN_SERVICE) {
numDecom--;
validDns.remove(dn);
+ LOG.warn("Cannot decommission " + dn.getHostName() + " because it is
not IN-SERVICE");
}
} catch (NodeNotFoundException ex) {
numDecom--;
validDns.remove(dn);
+ LOG.warn("Cannot decommission " + dn.getHostName() + " because it is
not found in SCM");
Review Comment:
```suggestion
LOG.warn("Cannot decommission {} because it is not found in SCM",
dn.getHostName());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
