sodonnel commented on code in PR #3744:
URL: https://github.com/apache/ozone/pull/3744#discussion_r971070115
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java:
##########
@@ -220,25 +158,171 @@ protected List<DatanodeDetails> chooseDatanodesInternal(
// If chosenNodes changed, reset the retryCount
retryCount = 0;
}
+ maxOuterLoopIterations--;
}
- List<DatanodeDetails> result = new ArrayList<>(chosenNodes);
- ContainerPlacementStatus placementStatus =
- validateContainerPlacement(result, nodesRequiredToChoose);
- if (!placementStatus.isPolicySatisfied()) {
- String errorMsg = "ContainerPlacementPolicy not met, currentRacks is " +
- placementStatus.actualPlacementCount() + "desired racks is" +
- placementStatus.expectedPlacementCount();
+ return chosenNodes;
+ }
+
+ /**
+ * Called by SCM to choose datanodes.
+ *
+ * @param usedNodes - list of the datanodes to already chosen in the
+ * pipeline.
+ * @param excludedNodes - list of the datanodes to exclude.
+ * @param favoredNodes - list of nodes preferred. This is a hint to the
+ * allocator, whether the favored nodes will be used
+ * depends on whether the nodes meets the allocator's
+ * requirement.
+ * @param nodesRequiredToChoose - number of datanodes required.
+ * @param dataSizeRequired - size required for the container.
+ * @param metadataSizeRequired - size required for Ratis metadata.
+ * @return List of datanodes.
+ * @throws SCMException SCMException
+ */
+ @Override
+ protected List<DatanodeDetails> chooseDatanodesInternal(
+ List<DatanodeDetails> usedNodes,
+ final List<DatanodeDetails> excludedNodes,
+ final List<DatanodeDetails> favoredNodes,
+ final int nodesRequiredToChoose, final long metadataSizeRequired,
+ final long dataSizeRequired) throws SCMException {
+ if (nodesRequiredToChoose <= 0) {
+ String errorMsg = "num of nodes required to choose should bigger" +
+ "than 0, but the given num is " + nodesRequiredToChoose;
throw new SCMException(errorMsg, null);
}
+ metrics.incrDatanodeRequestCount(nodesRequiredToChoose);
+ int nodesRequired = nodesRequiredToChoose;
+ int excludedNodesCount = excludedNodes == null ? 0 : excludedNodes.size();
+ List<Node> availableNodes = networkTopology.getNodes(
+ networkTopology.getMaxLevel());
+ int totalNodesCount = availableNodes.size();
+ if (excludedNodes != null) {
+ availableNodes.removeAll(excludedNodes);
+ }
+ if (availableNodes.size() < nodesRequired) {
+ throw new SCMException("No enough datanodes to choose. " +
+ "TotalNode = " + totalNodesCount +
+ " AvailableNode = " + availableNodes.size() +
+ " RequiredNode = " + nodesRequired +
+ " ExcludedNode = " + excludedNodesCount,
+ SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ }
+
+ List<DatanodeDetails> mutableFavoredNodes = new ArrayList<>();
+ if (favoredNodes != null) {
+ // Generate mutableFavoredNodes, only stores valid favoredNodes
+ for (DatanodeDetails datanodeDetails : favoredNodes) {
+ if (isValidNode(datanodeDetails, metadataSizeRequired,
+ dataSizeRequired)) {
+ mutableFavoredNodes.add(datanodeDetails);
+ }
+ }
+ Collections.shuffle(mutableFavoredNodes);
+ }
+ if (excludedNodes != null) {
+ mutableFavoredNodes.removeAll(excludedNodes);
+ }
+
+ if (usedNodes == null) {
+ usedNodes = Collections.emptyList();
+ }
+ List<Node> racks = getAllRacks();
+ Set<Node> usedRacks = usedNodes.stream()
+ .map(node -> networkTopology.getAncestor(node, RACK_LEVEL))
+ .filter(node -> node != null)
+ .collect(Collectors.toSet());
+ int requiredReplicationFactor = usedNodes.size() + nodesRequired;
+ int numberOfRacksRequired =
+ getRequiredRackCount(requiredReplicationFactor);
+ int additionalRacksRequired = numberOfRacksRequired - usedRacks.size();
+ if (nodesRequired < additionalRacksRequired) {
+ String reason = "Required nodes size: " + nodesRequired
+ + " is less than required number of racks to choose: "
+ + additionalRacksRequired + ".";
+ LOG.warn("Placement policy cannot choose the enough racks. {}"
+ + "Total number of Required Racks: {} Used Racks Count:"
+
+ " {}, Required Nodes count: {}",
+ reason, numberOfRacksRequired, usedRacks.size(), nodesRequired);
+ throw new SCMException(reason,
Review Comment:
There are a few new `throw new SCMException(reason,` here I think - are we
making this class more strict, in that if it cannot spread the nodes across
racks it will throw, or are we keeping it largely as it was?
--
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]
