siddhantsangwan commented on code in PR #5184:
URL: https://github.com/apache/ozone/pull/5184#discussion_r1293431568
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -481,16 +486,20 @@ public boolean isValidNode(DatanodeDetails
datanodeDetails,
if (datanodeInfo == null) {
LOG.error("Failed to find the DatanodeInfo for datanode {}",
datanodeDetails);
- } else {
- if (datanodeInfo.getNodeStatus().isNodeWritable() &&
- (hasEnoughSpace(datanodeInfo, metadataSizeRequired,
- dataSizeRequired))) {
- LOG.debug("Datanode {} is chosen. Required metadata size is {} and " +
- "required data size is {}",
- datanodeDetails, metadataSizeRequired, dataSizeRequired);
- return true;
- }
+ return false;
+ }
+ NodeStatus nodeStatus = datanodeInfo.getNodeStatus();
+ if (nodeStatus.isNodeWritable() &&
+ (hasEnoughSpace(datanodeInfo, metadataSizeRequired,
+ dataSizeRequired))) {
+ LOG.debug("Datanode {} is chosen. Required metadata size is {} and " +
+ "required data size is {} and NodeStatus is {}",
+ datanodeDetails, metadataSizeRequired, dataSizeRequired, nodeStatus);
+ return true;
}
+ LOG.debug("Datanode {} is not chosen. Required metadata size is {} and " +
Review Comment:
NIT: Do we need to wrap this one and line 495 above with a
`LOG.isDebugEnabled()` check? `toString()` for `NodeStatus` does string
concatenation using `+`. I suspect the compiler optimises this by using a
`StringBuilder`, but not sure.
```
public String toString() {
return "OperationalState: " + operationalState + " Health: " + health +
" OperationStateExpiry: " + opStateExpiryEpochSeconds;
}
```
It's only using existing variables so I don't think this is an expensive
computation anyway.
--
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]
