neils-dev commented on code in PR #4002: URL: https://github.com/apache/ozone/pull/4002#discussion_r1038525371
########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicyFactory.java: ########## @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.pipeline; + +import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.scm.PlacementPolicy; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementMetrics; +import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRackScatter; +import org.apache.hadoop.hdds.scm.exceptions.SCMException; +import org.apache.hadoop.hdds.scm.net.NetworkTopology; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; + +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CONTAINER_PLACEMENT_IMPL_KEY; + +/** + * Pipeline placement factor for pipeline providers to create placement instance + * based on configuration property. + * {@link ScmConfigKeys#OZONE_SCM_CONTAINER_PLACEMENT_IMPL_KEY} + */ +public final class PipelinePlacementPolicyFactory { + private static final Logger LOG = + LoggerFactory.getLogger(PipelinePlacementPolicyFactory.class); + + private static final Class<? extends PlacementPolicy> + OZONE_SCM_CONTAINER_PLACEMENT_IMPL_DEFAULT = + SCMContainerPlacementRackScatter.class; + + private PipelinePlacementPolicyFactory() { + } + + public static PlacementPolicy getPolicy( + ConfigurationSource conf, final NodeManager nodeManager, + final PipelineStateManager stateManager, + NetworkTopology clusterMap, final boolean fallback, + SCMContainerPlacementMetrics metrics) throws SCMException { + + Class<? extends PlacementPolicy> placementClass = + PipelinePlacementPolicy.class; + Constructor<? extends PlacementPolicy> constructor; + + try { + if (conf.get(OZONE_SCM_CONTAINER_PLACEMENT_IMPL_KEY) + .equals(SCMContainerPlacementRackScatter.class.getCanonicalName())) { + placementClass = conf.getClass(ScmConfigKeys + .OZONE_SCM_CONTAINER_PLACEMENT_IMPL_KEY, + OZONE_SCM_CONTAINER_PLACEMENT_IMPL_DEFAULT, + PlacementPolicy.class); + + constructor = placementClass.getDeclaredConstructor(NodeManager.class, + ConfigurationSource.class, NetworkTopology.class, boolean.class, + SCMContainerPlacementMetrics.class); + LOG.info("Create pipeline placement policy of type {}", + placementClass.getCanonicalName()); + return (constructor.newInstance(nodeManager, conf, clusterMap, + fallback, metrics)); + } + constructor = placementClass.getDeclaredConstructor(NodeManager.class, + PipelineStateManager.class, ConfigurationSource.class); + LOG.info("Create pipeline placement policy of type {}", + placementClass.getCanonicalName()); + return (constructor.newInstance(nodeManager, stateManager, conf)); Review Comment: Currently, the placement polices for pipeline placement differs from those for the replication manager container placement policies. Because of this, the pipeline policy factory as we have it now, explicitly checks the config for `SCMScaterPlacementRackScatter` set. If set, then we reuse the `SCMScatterPlacementRackScatter` for pipeline provider pipeline placement as we wanted to do, otherwise the factory uses the existing `PipelinePlacementPolicy` that differs in behavior from container placement policies. Differing aspects of (pipeline provider) `PipelinePlacementPolicy` to (replication manager) `SCMContainer` placement policies include, i.) container placement policies do not check for pipeline engagement (we are accommodating this with excluding nodes that have full pipeline engagements in the `RatisPipelineProvider` prior to calling `placementPolicy.chooseDatanodes()`). ii.) container placement policies enact `SCMContainerPlacementMetrics` that are not collected for pipeline provider placement iii.) pipeline provider placement policies, EXCEPT for our implementation support for `RackScatter`, are not configurable. Prior to our `RackScatter` implementation the supported pipeline provider pipeline placements, `Random` and `RackAware`, were chosen within the `PipelinePlacementPolicy` based upon whether the all nodes were on the same rack or not respectively. Currently, we are unable to directly select the placement policy with reflection for the pipeline provider as we do with the container provider. Should we continue to explicitly check for SCMContainerPlaementRackScatter here, or is there a way we can use reflection? -- 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]
