From common-commits-return-83516-archive-asf-public=cust-asf.ponee.io@hadoop.apache.org Wed May 30 23:12:06 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id F081618063B for ; Wed, 30 May 2018 23:12:05 +0200 (CEST) Received: (qmail 47765 invoked by uid 500); 30 May 2018 21:11:53 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 46774 invoked by uid 99); 30 May 2018 21:11:52 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 30 May 2018 21:11:52 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 55C18E117F; Wed, 30 May 2018 21:11:52 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: hanishakoneru@apache.org To: common-commits@hadoop.apache.org Date: Wed, 30 May 2018 21:12:38 -0000 Message-Id: <1fd99991625d49dd829cbab250ac87e6@git.apache.org> In-Reply-To: <5b4a60ac3248446a8cce49051bf843e2@git.apache.org> References: <5b4a60ac3248446a8cce49051bf843e2@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [48/50] [abbrv] hadoop git commit: YARN-8350. NPE in service AM related to placement policy. Contributed by Gour Saha YARN-8350. NPE in service AM related to placement policy. Contributed by Gour Saha Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/778a4a24 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/778a4a24 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/778a4a24 Branch: refs/heads/HDDS-48 Commit: 778a4a24be176382a5704f709c00bdfcfe6ddc8c Parents: 96eefcc Author: Billie Rinaldi Authored: Wed May 30 13:19:13 2018 -0700 Committer: Billie Rinaldi Committed: Wed May 30 13:19:13 2018 -0700 ---------------------------------------------------------------------- .../yarn/service/component/Component.java | 114 ++++++++++--------- .../exceptions/RestApiErrorMessages.java | 8 ++ .../yarn/service/utils/ServiceApiUtil.java | 24 +++- .../hadoop/yarn/service/TestServiceApiUtil.java | 44 ++++++- 4 files changed, 130 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/778a4a24/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/component/Component.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/component/Component.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/component/Component.java index 931877e..a1ee796 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/component/Component.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/component/Component.java @@ -694,62 +694,66 @@ public class Component implements EventHandler { // composite constraints then this AND-ed composite constraint is not // used. PlacementConstraint finalConstraint = null; - for (org.apache.hadoop.yarn.service.api.records.PlacementConstraint - yarnServiceConstraint : placementPolicy.getConstraints()) { - List targetExpressions = new ArrayList<>(); - // Currently only intra-application allocation tags are supported. - if (!yarnServiceConstraint.getTargetTags().isEmpty()) { - targetExpressions.add(PlacementTargets.allocationTag( - yarnServiceConstraint.getTargetTags().toArray(new String[0]))); - } - // Add all node attributes - for (Map.Entry> attribute : yarnServiceConstraint - .getNodeAttributes().entrySet()) { - targetExpressions.add(PlacementTargets.nodeAttribute( - attribute.getKey(), attribute.getValue().toArray(new String[0]))); - } - // Add all node partitions - if (!yarnServiceConstraint.getNodePartitions().isEmpty()) { - targetExpressions - .add(PlacementTargets.nodePartition(yarnServiceConstraint - .getNodePartitions().toArray(new String[0]))); - } - PlacementConstraint constraint = null; - switch (yarnServiceConstraint.getType()) { - case AFFINITY: - constraint = PlacementConstraints - .targetIn(yarnServiceConstraint.getScope().getValue(), - targetExpressions.toArray(new TargetExpression[0])) - .build(); - break; - case ANTI_AFFINITY: - constraint = PlacementConstraints - .targetNotIn(yarnServiceConstraint.getScope().getValue(), - targetExpressions.toArray(new TargetExpression[0])) - .build(); - break; - case AFFINITY_WITH_CARDINALITY: - constraint = PlacementConstraints.targetCardinality( - yarnServiceConstraint.getScope().name().toLowerCase(), - yarnServiceConstraint.getMinCardinality() == null ? 0 - : yarnServiceConstraint.getMinCardinality().intValue(), - yarnServiceConstraint.getMaxCardinality() == null - ? Integer.MAX_VALUE - : yarnServiceConstraint.getMaxCardinality().intValue(), - targetExpressions.toArray(new TargetExpression[0])).build(); - break; - } - // The default AND-ed final composite constraint - if (finalConstraint != null) { - finalConstraint = PlacementConstraints - .and(constraint.getConstraintExpr(), - finalConstraint.getConstraintExpr()) - .build(); - } else { - finalConstraint = constraint; + if (placementPolicy != null) { + for (org.apache.hadoop.yarn.service.api.records.PlacementConstraint + yarnServiceConstraint : placementPolicy.getConstraints()) { + List targetExpressions = new ArrayList<>(); + // Currently only intra-application allocation tags are supported. + if (!yarnServiceConstraint.getTargetTags().isEmpty()) { + targetExpressions.add(PlacementTargets.allocationTag( + yarnServiceConstraint.getTargetTags().toArray(new String[0]))); + } + // Add all node attributes + for (Map.Entry> attribute : yarnServiceConstraint + .getNodeAttributes().entrySet()) { + targetExpressions + .add(PlacementTargets.nodeAttribute(attribute.getKey(), + attribute.getValue().toArray(new String[0]))); + } + // Add all node partitions + if (!yarnServiceConstraint.getNodePartitions().isEmpty()) { + targetExpressions + .add(PlacementTargets.nodePartition(yarnServiceConstraint + .getNodePartitions().toArray(new String[0]))); + } + PlacementConstraint constraint = null; + switch (yarnServiceConstraint.getType()) { + case AFFINITY: + constraint = PlacementConstraints + .targetIn(yarnServiceConstraint.getScope().getValue(), + targetExpressions.toArray(new TargetExpression[0])) + .build(); + break; + case ANTI_AFFINITY: + constraint = PlacementConstraints + .targetNotIn(yarnServiceConstraint.getScope().getValue(), + targetExpressions.toArray(new TargetExpression[0])) + .build(); + break; + case AFFINITY_WITH_CARDINALITY: + constraint = PlacementConstraints.targetCardinality( + yarnServiceConstraint.getScope().name().toLowerCase(), + yarnServiceConstraint.getMinCardinality() == null ? 0 + : yarnServiceConstraint.getMinCardinality().intValue(), + yarnServiceConstraint.getMaxCardinality() == null + ? Integer.MAX_VALUE + : yarnServiceConstraint.getMaxCardinality().intValue(), + targetExpressions.toArray(new TargetExpression[0])).build(); + break; + } + // The default AND-ed final composite constraint + if (finalConstraint != null) { + finalConstraint = PlacementConstraints + .and(constraint.getConstraintExpr(), + finalConstraint.getConstraintExpr()) + .build(); + } else { + finalConstraint = constraint; + } + LOG.debug("[COMPONENT {}] Placement constraint: {}", + componentSpec.getName(), + constraint.getConstraintExpr().toString()); } - LOG.debug("[COMPONENT {}] Placement constraint: {}", - componentSpec.getName(), constraint.getConstraintExpr().toString()); } ResourceSizing resourceSizing = ResourceSizing.newInstance((int) count, resource); http://git-wip-us.apache.org/repos/asf/hadoop/blob/778a4a24/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java index 5b6eac3..1d2d719 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java @@ -91,6 +91,14 @@ public interface RestApiErrorMessages { String ERROR_QUICKLINKS_FOR_COMP_INVALID = "Quicklinks specified at" + " component level, needs corresponding values set at service level"; + // Note: %sin is not a typo. Constraint name is optional so the error messages + // below handle that scenario by adding a space if name is specified. + String ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL = "Type not specified " + + "for constraint %sin placement policy of component %s."; + String ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL = "Scope not specified " + + "for constraint %sin placement policy of component %s."; + String ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL = "Tag(s) not specified " + + "for constraint %sin placement policy of component %s."; String ERROR_PLACEMENT_POLICY_TAG_NAME_NOT_SAME = "Invalid target tag %s " + "specified in placement policy of component %s. For now, target tags " + "support self reference only. Specifying anything other than its " http://git-wip-us.apache.org/repos/asf/hadoop/blob/778a4a24/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java index 2f826fa..6101bf0 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java @@ -40,6 +40,7 @@ import org.apache.hadoop.yarn.service.api.records.Component; import org.apache.hadoop.yarn.service.api.records.Configuration; import org.apache.hadoop.yarn.service.api.records.KerberosPrincipal; import org.apache.hadoop.yarn.service.api.records.PlacementConstraint; +import org.apache.hadoop.yarn.service.api.records.PlacementPolicy; import org.apache.hadoop.yarn.service.api.records.Resource; import org.apache.hadoop.yarn.service.exceptions.SliderException; import org.apache.hadoop.yarn.service.conf.RestApiConstants; @@ -314,9 +315,28 @@ public class ServiceApiUtil { private static void validatePlacementPolicy(List components, Set componentNames) { for (Component comp : components) { - if (comp.getPlacementPolicy() != null) { - for (PlacementConstraint constraint : comp.getPlacementPolicy() + PlacementPolicy placementPolicy = comp.getPlacementPolicy(); + if (placementPolicy != null) { + for (PlacementConstraint constraint : placementPolicy .getConstraints()) { + if (constraint.getType() == null) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } + if (constraint.getScope() == null) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } + if (constraint.getTargetTags().isEmpty()) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } for (String targetTag : constraint.getTargetTags()) { if (!comp.getName().equals(targetTag)) { throw new IllegalArgumentException(String.format( http://git-wip-us.apache.org/repos/asf/hadoop/blob/778a4a24/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java index b209bbb..243c6b3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java @@ -25,6 +25,8 @@ import org.apache.hadoop.yarn.service.api.records.Component; import org.apache.hadoop.yarn.service.api.records.KerberosPrincipal; import org.apache.hadoop.yarn.service.api.records.PlacementConstraint; import org.apache.hadoop.yarn.service.api.records.PlacementPolicy; +import org.apache.hadoop.yarn.service.api.records.PlacementScope; +import org.apache.hadoop.yarn.service.api.records.PlacementType; import org.apache.hadoop.yarn.service.api.records.Resource; import org.apache.hadoop.yarn.service.api.records.Service; import org.apache.hadoop.yarn.service.exceptions.RestApiErrorMessages; @@ -503,13 +505,48 @@ public class TestServiceApiUtil { PlacementPolicy pp = new PlacementPolicy(); PlacementConstraint pc = new PlacementConstraint(); pc.setName("CA1"); - pc.setTargetTags(Collections.singletonList("comp-invalid")); pp.setConstraints(Collections.singletonList(pc)); comp.setPlacementPolicy(pp); try { ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); - Assert.fail(EXCEPTION_PREFIX + "service with empty placement"); + Assert.fail(EXCEPTION_PREFIX + "constraint with no type"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set the type + pc.setType(PlacementType.ANTI_AFFINITY); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with no scope"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set the scope + pc.setScope(PlacementScope.NODE); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with no tag(s)"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set a target tag - but an invalid one + pc.setTargetTags(Collections.singletonList("comp-invalid")); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with invalid tag name"); } catch (IllegalArgumentException e) { assertEquals( String.format( @@ -518,9 +555,10 @@ public class TestServiceApiUtil { e.getMessage()); } + // Set valid target tags now pc.setTargetTags(Collections.singletonList("comp-a")); - // now it should succeed + // Finally it should succeed try { ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); } catch (IllegalArgumentException e) { --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org