bipinprasad commented on a change in pull request #3215: Storm3585 - New 
compact Constraint config including maxCoLocationCnt
URL: https://github.com/apache/storm/pull/3215#discussion_r387895854
 
 

 ##########
 File path: 
storm-server/src/main/java/org/apache/storm/scheduler/resource/strategies/scheduling/ConstraintSolverStrategy.java
 ##########
 @@ -38,71 +39,188 @@
 import org.apache.storm.shade.com.google.common.annotations.VisibleForTesting;
 import org.apache.storm.utils.ObjectReader;
 import org.apache.storm.utils.Time;
+import org.apache.storm.validation.ConfigValidation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class ConstraintSolverStrategy extends BaseResourceAwareStrategy {
-    //hard coded max number of states to search
     private static final Logger LOG = 
LoggerFactory.getLogger(ConstraintSolverStrategy.class);
 
-    //constraints and spreads
-    private Map<String, Map<String, Integer>> constraintMatrix;
-    private HashSet<String> spreadComps = new HashSet<>();
+    public static final String CONSTRAINT_TYPE_MAX_NODE_CO_LOCATION_CNT = 
"maxNodeCoLocationCnt";
+    public static final String CONSTRAINT_TYPE_INCOMPATIBLE_COMPONENTS = 
"incompatibleComponents";
 
-    private Map<String, RasNode> nodes;
-    private Map<ExecutorDetails, String> execToComp;
-    private Map<String, Set<ExecutorDetails>> compToExecs;
-    private List<String> favoredNodeIds;
-    private List<String> unFavoredNodeIds;
+    /**
+     * Component constraint as derived from configuration.
+     * This is backward compatible and can parse old style 
Config.TOPOLOGY_RAS_CONSTRAINTS and Config.TOPOLOGY_SPREAD_COMPONENTS.
+     * New style Config.TOPOLOGY_RAS_CONSTRAINTS is map where each component 
has a list of other incompatible components
+     * and an optional number that specifies the maximum co-location count for 
the component on a node.
+     *
+     * <p>comp-1 cannot exist on same worker as comp-2 or comp-3, and at most 
"2" comp-1 on same node</p>
+     * <p>comp-2 and comp-4 cannot be on same node (missing comp-1 is implied 
from comp-1 constraint)</p>
+     *
+     *  <p>
+     *      { "comp-1": { "maxNodeCoLocationCnt": 2, "incompatibleComponents": 
["comp-2", "comp-3" ] },
+     *        "comp-2": { "incompatibleComponents": [ "comp-4" ] }
+     *      }
+     *  </p>
+     */
+    public static final class ConstraintConfig {
+        private Map<String, Set<String>> incompatibleComponents = new 
HashMap<>();
+        private Map<String, Integer> maxCoLocationCnts = new HashMap<>(); // 
maximum node CoLocationCnt for restricted components
 
-    static Map<String, Map<String, Integer>> getConstraintMap(TopologyDetails 
topo, Set<String> comps) {
-        Map<String, Map<String, Integer>> matrix = new HashMap<>();
-        for (String comp : comps) {
-            matrix.put(comp, new HashMap<>());
-            for (String comp2 : comps) {
-                matrix.get(comp).put(comp2, 0);
-            }
+        ConstraintConfig(TopologyDetails topo) {
+            this(topo.getConf(), topo.getComponents().keySet());
         }
-        List<List<String>> constraints = (List<List<String>>) 
topo.getConf().get(Config.TOPOLOGY_RAS_CONSTRAINTS);
-        if (constraints != null) {
-            for (List<String> constraintPair : constraints) {
-                String comp1 = constraintPair.get(0);
-                String comp2 = constraintPair.get(1);
-                if (!matrix.containsKey(comp1)) {
-                    LOG.warn("Comp: {} declared in constraints is not valid!", 
comp1);
-                    continue;
+
+        ConstraintConfig(Map<String, Object> conf, Set<String> comps) {
+            // spread constraints, old style may come from 
Config.TOPOLOGY_RAS_CONSTRAINTS where the target
+            Object rasConstraints = conf.get(Config.TOPOLOGY_RAS_CONSTRAINTS);
+            comps.forEach(k -> incompatibleComponents.computeIfAbsent(k, x -> 
new HashSet<>()));
+            if (rasConstraints instanceof List) {
+                // old style
+                List<List<String>> constraints = (List<List<String>>) 
rasConstraints;
+                for (List<String> constraintPair : constraints) {
+                    String comp1 = constraintPair.get(0);
+                    String comp2 = constraintPair.get(1);
+                    if (!comps.contains(comp1)) {
+                        LOG.warn("Comp: {} declared in constraints is not 
valid!", comp1);
+                        continue;
+                    }
+                    if (!comps.contains(comp2)) {
+                        LOG.warn("Comp: {} declared in constraints is not 
valid!", comp2);
+                        continue;
+                    }
+                    if (comp1.equals(comp2)) {
+                        maxCoLocationCnts.put(comp1, 1);
+                    } else {
+                        incompatibleComponents.get(comp1).add(comp2);
+                        incompatibleComponents.get(comp2).add(comp1);
+                    }
                 }
-                if (!matrix.containsKey(comp2)) {
-                    LOG.warn("Comp: {} declared in constraints is not valid!", 
comp2);
-                    continue;
+            } else {
+                Map<String, Map<String,?>> constraintMap = (Map<String, 
Map<String,?>>) rasConstraints;
+                constraintMap.forEach((comp1, v) -> {
+                    if (comps.contains(comp1)) {
+                        // v is a list of other components and an optional 
number which is a maxCoLocationCnt
+                        v.forEach((ctype, constraint) -> {
+                            switch (ctype) {
+                                case CONSTRAINT_TYPE_MAX_NODE_CO_LOCATION_CNT:
+                                    try {
+                                        int numValue = Integer.parseInt("" + 
constraint);
+                                        if (numValue < 1) {
+                                            LOG.warn("{} {} declared for Comp 
{} is not valid, expected >= 1", ctype, numValue, comp1);
+                                        } else {
+                                            maxCoLocationCnts.put(comp1, 
numValue);
+                                        }
+                                    } catch (Exception ex) {
+                                        LOG.warn("{} {} declared for Comp {} 
is not valid, expected >= 1", ctype, constraint, comp1);
+                                    }
+                                    break;
+
+                                case CONSTRAINT_TYPE_INCOMPATIBLE_COMPONENTS:
+                                    if (!(constraint instanceof List || 
constraint instanceof String)) {
+                                        LOG.warn("{} {} declared for Comp {} 
is not valid, expecting a list of components or 1 component",
+                                                ctype, constraint, comp1);
+                                        break;
+                                    }
+                                    List<String> list;
+                                    list = (constraint instanceof String) ? 
Arrays.asList((String)constraint) : (List<String>)constraint;
+                                    for (String comp2: list) {
+                                        if (!comps.contains(comp2)) {
+                                            LOG.warn("{} {} declared for Comp 
{} is not a valid component", ctype, comp2, comp1);
+                                            continue;
+                                        }
+                                        if (comp1.equals(comp2)) {
+                                            if 
(!maxCoLocationCnts.containsKey(comp1)) {
+                                                maxCoLocationCnts.put(comp1, 
1);
+                                            }
+                                        } else {
+                                            
incompatibleComponents.get(comp1).add(comp2);
+                                            
incompatibleComponents.get(comp2).add(comp1);
+                                        }
+                                    }
+                                    break;
+
+                                default:
+                                    LOG.warn("ConstraintType={} invalid for 
component={}, valid values are {} and {}, ignoring value={}",
+                                            ctype, comp1, 
CONSTRAINT_TYPE_MAX_NODE_CO_LOCATION_CNT,
+                                            
CONSTRAINT_TYPE_INCOMPATIBLE_COMPONENTS, constraint);
+                                    break;
+                            }
+                        });
+                    }
+                });
+            }
+
+            // process Config.TOPOLOGY_SPREAD_COMPONENTS - old style
+            Object obj = conf.get(Config.TOPOLOGY_SPREAD_COMPONENTS);
 
 Review comment:
   Changed code to use only if maxNodeCoLocationCnt is not already defined in 
TOPOLOGY_RAS_CONSTRAINTS., warn otherwise.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to