YARN-7709. Remove SELF from TargetExpression type. (Konstantinos Karanasos via asuresh)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/8779a357 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/8779a357 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/8779a357 Branch: refs/heads/YARN-6592 Commit: 8779a35742085fadddccc21342b55d4f17fae5c2 Parents: 29d9e4d Author: Arun Suresh <asur...@apache.org> Authored: Thu Jan 18 04:29:57 2018 -0800 Committer: Arun Suresh <asur...@apache.org> Committed: Wed Jan 31 01:30:17 2018 -0800 ---------------------------------------------------------------------- .../yarn/api/resource/PlacementConstraint.java | 32 ++++++++++++++---- .../yarn/api/resource/PlacementConstraints.java | 35 +++++++++----------- .../api/resource/TestPlacementConstraints.java | 3 +- .../PlacementConstraintTransformations.java | 19 +++-------- .../TestPlacementConstraintTransformations.java | 35 +++++--------------- .../constraint/PlacementConstraintsUtil.java | 10 ++++-- 6 files changed, 64 insertions(+), 70 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraint.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraint.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraint.java index b6e851a..4d998ac 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraint.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraint.java @@ -242,7 +242,7 @@ public class PlacementConstraint { * Enum specifying the type of the target expression. */ public enum TargetType { - NODE_ATTRIBUTE, ALLOCATION_TAG, SELF + NODE_ATTRIBUTE, ALLOCATION_TAG } private TargetType targetType; @@ -418,23 +418,25 @@ public class PlacementConstraint { } /** - * Class that represents a cardinality constraint. Such a constraint the - * number of allocations within a given scope to some minimum and maximum - * values. + * Class that represents a cardinality constraint. Such a constraint allows + * the number of allocations with a specific set of tags and within a given + * scope to be between some minimum and maximum values. * * It is a specialized version of the {@link SingleConstraint}, where the - * target is self (i.e., the allocation to which the constraint is attached). + * target is a set of allocation tags. */ public static class CardinalityConstraint extends AbstractConstraint { private String scope; private int minCardinality; private int maxCardinality; + private Set<String> allocationTags; public CardinalityConstraint(String scope, int minCardinality, - int maxCardinality) { + int maxCardinality, Set<String> allocationTags) { this.scope = scope; this.minCardinality = minCardinality; this.maxCardinality = maxCardinality; + this.allocationTags = allocationTags; } /** @@ -464,11 +466,21 @@ public class PlacementConstraint { return maxCardinality; } + /** + * Get the allocation tags of the constraint. + * + * @return the allocation tags of the constraint + */ + public Set<String> getAllocationTags() { + return allocationTags; + } + @Override public <T> T accept(Visitor<T> visitor) { return visitor.visit(this); } + @Override public boolean equals(Object o) { if (this == o) { @@ -486,7 +498,11 @@ public class PlacementConstraint { if (maxCardinality != that.maxCardinality) { return false; } - return scope != null ? scope.equals(that.scope) : that.scope == null; + if (scope != null ? !scope.equals(that.scope) : that.scope != null) { + return false; + } + return allocationTags != null ? allocationTags.equals(that.allocationTags) + : that.allocationTags == null; } @Override @@ -494,6 +510,8 @@ public class PlacementConstraint { int result = scope != null ? scope.hashCode() : 0; result = 31 * result + minCardinality; result = 31 * result + maxCardinality; + result = 31 * result + + (allocationTags != null ? allocationTags.hashCode() : 0); return result; } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraints.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraints.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraints.java index 8e84280..c8991cb 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraints.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraints.java @@ -95,40 +95,45 @@ public final class PlacementConstraints { * the scope * @param maxCardinality determines the maximum number of allocations within * the scope + * @param allocationTags the constraint targets allocations with these tags * @return the resulting placement constraint */ public static AbstractConstraint cardinality(String scope, int minCardinality, - int maxCardinality) { + int maxCardinality, String... allocationTags) { return new SingleConstraint(scope, minCardinality, maxCardinality, - PlacementTargets.self()); + PlacementTargets.allocationTag(allocationTags)); } /** - * Similar to {@link #cardinality(String, int, int)}, but determines only the - * minimum cardinality (the maximum cardinality is unbound). + * Similar to {@link #cardinality(String, int, int, String...)}, but + * determines only the minimum cardinality (the maximum cardinality is + * unbound). * * @param scope the scope of the constraint * @param minCardinality determines the minimum number of allocations within * the scope + * @param allocationTags the constraint targets allocations with these tags * @return the resulting placement constraint */ public static AbstractConstraint minCardinality(String scope, - int minCardinality) { - return cardinality(scope, minCardinality, Integer.MAX_VALUE); + int minCardinality, String... allocationTags) { + return cardinality(scope, minCardinality, Integer.MAX_VALUE, + allocationTags); } /** - * Similar to {@link #cardinality(String, int, int)}, but determines only the - * maximum cardinality (the minimum can be as low as 0). + * Similar to {@link #cardinality(String, int, int, String...)}, but + * determines only the maximum cardinality (the minimum can be as low as 0). * * @param scope the scope of the constraint * @param maxCardinality determines the maximum number of allocations within * the scope + * @param allocationTags the constraint targets allocations with these tags * @return the resulting placement constraint */ public static AbstractConstraint maxCardinality(String scope, - int maxCardinality) { - return cardinality(scope, 0, maxCardinality); + int maxCardinality, String... allocationTags) { + return cardinality(scope, 0, maxCardinality, allocationTags); } /** @@ -193,16 +198,6 @@ public final class PlacementConstraints { return new TargetExpression(TargetType.ALLOCATION_TAG, null, allocationTags); } - - /** - * The default target expression that uses as target the allocation that - * specifies the constraint. - * - * @return the self-target - */ - public static TargetExpression self() { - return new TargetExpression(TargetType.SELF); - } } // Creation of compound constraints. http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraints.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraints.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraints.java index e25d477..2f8cc62 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraints.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraints.java @@ -86,7 +86,8 @@ public class TestPlacementConstraints { @Test public void testAndConstraint() { AbstractConstraint constraintExpr = - and(targetIn(RACK, allocationTag("spark")), maxCardinality(NODE, 3), + and(targetIn(RACK, allocationTag("spark")), + maxCardinality(NODE, 3, "spark"), targetCardinality(RACK, 2, 10, allocationTag("zk"))); And andExpr = (And) constraintExpr; http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraintTransformations.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraintTransformations.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraintTransformations.java index e9eda6f..c5d21af 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraintTransformations.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/resource/PlacementConstraintTransformations.java @@ -162,15 +162,16 @@ public class PlacementConstraintTransformations { public AbstractConstraint visit(CardinalityConstraint constraint) { return new SingleConstraint(constraint.getScope(), constraint.getMinCardinality(), constraint.getMaxCardinality(), - new TargetExpression(TargetExpression.TargetType.SELF)); + new TargetExpression(TargetExpression.TargetType.ALLOCATION_TAG, null, + constraint.getAllocationTags())); } } /** * Visits a {@link PlacementConstraint} tree and, whenever possible, - * substitutes each {@link SingleConstraint} with a {@link TargetConstraint} - * or a {@link CardinalityConstraint}. When such a substitution is not - * possible, we keep the original {@link SingleConstraint}. + * substitutes each {@link SingleConstraint} with a {@link TargetConstraint}. + * When such a substitution is not possible, we keep the original + * {@link SingleConstraint}. */ public static class SpecializedConstraintTransformer extends AbstractTransformer { @@ -182,16 +183,6 @@ public class PlacementConstraintTransformations { @Override public AbstractConstraint visit(SingleConstraint constraint) { AbstractConstraint transformedConstraint = constraint; - // Check if it is a cardinality constraint. - if (constraint.getTargetExpressions().size() == 1) { - TargetExpression targetExpr = - constraint.getTargetExpressions().iterator().next(); - if (targetExpr.getTargetType() == TargetExpression.TargetType.SELF) { - transformedConstraint = new CardinalityConstraint( - constraint.getScope(), constraint.getMinCardinality(), - constraint.getMaxCardinality()); - } - } // Check if it is a target constraint. if (constraint.getMinCardinality() == 1 && constraint.getMaxCardinality() == Integer.MAX_VALUE) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraintTransformations.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraintTransformations.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraintTransformations.java index 1763735..62da092 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraintTransformations.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/resource/TestPlacementConstraintTransformations.java @@ -20,7 +20,6 @@ package org.apache.hadoop.yarn.api.resource; import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.NODE; import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.RACK; -import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.cardinality; import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.maxCardinality; import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.or; import static org.apache.hadoop.yarn.api.resource.PlacementConstraints.targetCardinality; @@ -89,42 +88,26 @@ public class TestPlacementConstraintTransformations { @Test public void testCardinalityConstraint() { - AbstractConstraint sConstraintExpr = cardinality(RACK, 3, 10); - Assert.assertTrue(sConstraintExpr instanceof SingleConstraint); - PlacementConstraint sConstraint = - PlacementConstraints.build(sConstraintExpr); - - // Transform from SimpleConstraint to specialized CardinalityConstraint - SpecializedConstraintTransformer specTransformer = - new SpecializedConstraintTransformer(sConstraint); - PlacementConstraint cConstraint = specTransformer.transform(); - - AbstractConstraint cConstraintExpr = cConstraint.getConstraintExpr(); - Assert.assertTrue(cConstraintExpr instanceof CardinalityConstraint); - - SingleConstraint single = (SingleConstraint) sConstraintExpr; - CardinalityConstraint cardinality = (CardinalityConstraint) cConstraintExpr; - Assert.assertEquals(single.getScope(), cardinality.getScope()); - Assert.assertEquals(single.getMinCardinality(), - cardinality.getMinCardinality()); - Assert.assertEquals(single.getMaxCardinality(), - cardinality.getMaxCardinality()); + CardinalityConstraint cardinality = new CardinalityConstraint(RACK, 3, 10, + new HashSet<>(Arrays.asList("hb"))); + PlacementConstraint cConstraint = PlacementConstraints.build(cardinality); // Transform from specialized CardinalityConstraint to SimpleConstraint SingleConstraintTransformer singleTransformer = new SingleConstraintTransformer(cConstraint); - sConstraint = singleTransformer.transform(); + PlacementConstraint sConstraint = singleTransformer.transform(); - sConstraintExpr = sConstraint.getConstraintExpr(); + AbstractConstraint sConstraintExpr = sConstraint.getConstraintExpr(); Assert.assertTrue(sConstraintExpr instanceof SingleConstraint); - single = (SingleConstraint) sConstraintExpr; + SingleConstraint single = (SingleConstraint) sConstraintExpr; Assert.assertEquals(cardinality.getScope(), single.getScope()); Assert.assertEquals(cardinality.getMinCardinality(), single.getMinCardinality()); Assert.assertEquals(cardinality.getMaxCardinality(), single.getMaxCardinality()); - Assert.assertEquals(new HashSet<>(Arrays.asList(PlacementTargets.self())), + Assert.assertEquals( + new HashSet<>(Arrays.asList(PlacementTargets.allocationTag("hb"))), single.getTargetExpressions()); } @@ -166,7 +149,7 @@ public class TestPlacementConstraintTransformations { List<AbstractConstraint> specChildren = specOrExpr.getChildren(); Assert.assertEquals(3, specChildren.size()); Assert.assertTrue(specChildren.get(0) instanceof TargetConstraint); - Assert.assertTrue(specChildren.get(1) instanceof CardinalityConstraint); + Assert.assertTrue(specChildren.get(1) instanceof SingleConstraint); Assert.assertTrue(specChildren.get(2) instanceof SingleConstraint); // Transform from specialized TargetConstraint to SimpleConstraint http://git-wip-us.apache.org/repos/asf/hadoop/blob/8779a357/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/constraint/PlacementConstraintsUtil.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/constraint/PlacementConstraintsUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/constraint/PlacementConstraintsUtil.java index c4b82e8..73b4f9e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/constraint/PlacementConstraintsUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/constraint/PlacementConstraintsUtil.java @@ -24,10 +24,10 @@ import org.apache.hadoop.classification.InterfaceAudience.Public; import org.apache.hadoop.classification.InterfaceStability.Unstable; import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.api.resource.PlacementConstraint; -import org.apache.hadoop.yarn.api.resource.PlacementConstraint.TargetExpression; -import org.apache.hadoop.yarn.api.resource.PlacementConstraint.TargetExpression.TargetType; import org.apache.hadoop.yarn.api.resource.PlacementConstraint.AbstractConstraint; import org.apache.hadoop.yarn.api.resource.PlacementConstraint.SingleConstraint; +import org.apache.hadoop.yarn.api.resource.PlacementConstraint.TargetExpression; +import org.apache.hadoop.yarn.api.resource.PlacementConstraint.TargetExpression.TargetType; import org.apache.hadoop.yarn.api.resource.PlacementConstraintTransformations.SingleConstraintTransformer; import org.apache.hadoop.yarn.api.resource.PlacementConstraints; import org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode; @@ -118,6 +118,12 @@ public final class PlacementConstraintsUtil { TargetExpression currentExp = expIt.next(); // Supporting AllocationTag Expressions for now if (currentExp.getTargetType().equals(TargetType.ALLOCATION_TAG)) { + // If source and tag allocation tags are the same, we do not enforce + // constraints with minimum cardinality. + if (currentExp.getTargetValues().equals(allocationTags) + && single.getMinCardinality() > 0) { + return true; + } // Check if conditions are met if (!canSatisfySingleConstraintExpression(appId, single, currentExp, node, tagsManager)) { --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
