devmadhuu commented on code in PR #11917:
URL: https://github.com/apache/ozone/pull/11917#discussion_r4482490650
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java:
##########
@@ -127,6 +127,19 @@ protected List<DatanodeDetails> chooseDatanodesInternal(
*/
@Override
public DatanodeDetails chooseNode(List<DatanodeDetails> healthyNodes) {
+ return chooseNode(healthyNodes, null);
Review Comment:
Thanks, I agree that a named comparison mode could make the intent clearer.
Here, null specifically means comparing overall datanode usage, rather than
selecting an empty or default storage type.
`StorageType` is Hadoop’s enum, so we cannot add NONE to it within Ozone. We
could introduce an Ozone-owned internal comparison abstraction, but that would
be separate from the physical storage types accepted by the existing placement
and metric APIs.
The current feature branch already uses null for aggregate statistics in
`SCMNodeMetric` and `SCMNodeStat`. I suggest retaining that contract in this
focused PR and documenting it clearly in the placement method and extracted
helper. `StorageType.DEFAULT` would not be equivalent because it means DISK.
If we want a named representation for aggregate comparison, we can design
that consistently across placement and statistics in a separate change.
--
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]
