sodonnel commented on PR #3606:
URL: https://github.com/apache/ozone/pull/3606#issuecomment-1188976178
While the change here will work, I do wonder if the same problem exists in
other placement policies. I also wonder if a similar problem could exist with
favoredNodes, although we don't seem to use it right now, at least in the EC
flow.
Would it be better to provide a new method on SCMNodeManager (and its
interface) to lookup the network location for a node that can be used within
the Common Placement Policy, as it already has a reference to NodeManager.
NodeManager already has the logic to resolve a host when it registers (see the
register method).
Then in SCMCommonPlacementPolicy, change the method:
```
@Override
public List<DatanodeDetails> chooseDatanodes(
List<DatanodeDetails> excludedNodes, List<DatanodeDetails>
favoredNodes,
int nodesRequired, long metadataSizeRequired, long dataSizeRequired)
throws SCMException {
```
To look like:
```
public List<DatanodeDetails> final chooseDatanodes(
List<DatanodeDetails> excludedNodes, List<DatanodeDetails>
favoredNodes,
int nodesRequired, long metadataSizeRequired, long dataSizeRequired)
throws SCMException {
fixedFavoredNodes = setNetworkLocation(favoredNodes);
fixedExcludedNodes = setNetworkLocation(excludedNodes);
chooseDatanodesInternal(...);
}
protected List<DatanodeDetails> chooseDatanodesInternal(....) {
// existing logic the original chooseDatanode
// Sub-classes must override here to provide their own implementation
of
// chooseDatanodes, ensuring the favored / excluded node lists always
have their
// network location resolved.
}
```
This way, each implementation must override the protected method and the
common code will always get executed. It would be even better if we could make
chooseDatanodesInternal abstract so they must provide an implementation, but I
am not sure we can do that with the way things are structured, as we have
Interface -> CommonImpl -> Other impls.
--
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]
