sodonnel commented on a change in pull request #881:
URL: https://github.com/apache/hadoop-ozone/pull/881#discussion_r420136338
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java
##########
@@ -198,4 +211,69 @@ public boolean hasEnoughSpace(DatanodeDetails
datanodeDetails,
*/
public abstract DatanodeDetails chooseNode(
List<DatanodeDetails> healthyNodes);
+
+ /**
+ * Default implementation for basic placement policies that do not have a
+ * placement policy. If the policy has not network topology this method
should
+ * return null.
+ * @return The networkTopology for the policy or null if none is configured.
+ */
+ protected NetworkTopology getNetworkTopology() {
Review comment:
The constructor for PipelinePlacementPolicy only receives nodeManager:
``` public PipelinePlacementPolicy(final NodeManager nodeManager,
final PipelineStateManager stateManager, final ConfigurationSource
conf) {
```
But the container one receives a networkTopology as part of its constructor:
```
public SCMContainerPlacementRackAware(final NodeManager nodeManager,
final ConfigurationSource conf, final NetworkTopology networkTopology,
final boolean fallback, final SCMContainerPlacementMetrics metrics) {
```
The pipline Policy always asks the nodeManager for the policy, but the
Container one uses the one passed. Ideally they should be the same, but I have
a feeling some tests may make use of this "feature".
The other old placement policies (SCMContainerPlacementRandom,
SCMContainerPlacementCapacity) have the same constructors, but they don't even
use the topology at all.
So, I think we can get rid of this new method and then we should create
another Jira to do the small refactor on all those older constructors to remove
network topology from the parameter list and ensure they pull the topology from
the node manager - what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
