neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999937402
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
For the pipeline placement, the following seems to be expected when choosing
nodes, first check healthy nodes, remove excluded and others, then check size
and return finalized list. This is followed in the comments of
`filterViableNodes
`https://github.com/apache/ozone/blob/fc0854b391ddc7716215907f7915f4b3f88211d6/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java#L109
and in the `SCMCommonPlacementPolicy.chooseDatanodesInternal` and looks
reasonable to follow here.
For the testcase,` testPipelineCreationOnNodeRestart()`, there are no
datanodes, so the check for healthynodes should raise an exception **prior** to
check for sufficient size raising the `FAILED_TO_FIND_NODES_WITH_SPACE`
exception. The healthynode exception may actually be something other then
`FAILED_TO_FIND_SUITABLE_NODE`, and perhaps
`SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES` to be consistent with
the exception raised in `SCMCommonPlacementPolicy.chooseDatanodesInternal` ?
Something similar to,
```
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
if (healthyNodes.size() == 0) {
msg = "No healthy node found to allocate container.";
LOG.error(msg);
throw new SCMException(msg, SCMException.ResultCodes
.FAILED_TO_FIND_HEALTHY_NODES);
```
?
--
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]
