diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java index 7f86cae..1c958e3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java @@ -253,24 +253,27 @@ public static boolean isAnyLocation(String hostName) { /** * Get node-label-expression for this Resource Request. If this is set, all * containers allocated to satisfy this resource-request will be only on those - * nodes that satisfy this node-label-expression + * nodes that satisfy this node-label-expression. + * + * Please note that node label expression now can only take effect when the + * resource request has resourceName = ANY * * @return node-label-expression */ @Public @Evolving - public abstract String getNodeLabelExpression(); + public abstract String getNodeLabelExpression(); /** - * Set node label expression of this resource request. Now only - * support AND(&&), in the future will provide support for OR(||), NOT(!). + * Set node label expression of this resource request. Now only support + * specifying a single node label. In the future we will support more complex + * node label expression specification like AND(&&), OR(||), etc. * - * Examples: - * - GPU && LARGE_MEM, ask for node has label GPU and LARGE_MEM together - * - "" (empty) means ask for node doesn't have label on it, this is default - * behavior + * Any please note that node label expression now can only take effect when + * the resource request has resourceName = ANY * - * @param nodelabelExpression node-label-expression of this ResourceRequest + * @param nodelabelExpression + * node-label-expression of this ResourceRequest */ @Public @Evolving diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java index 6f8c65a..9923806 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java @@ -169,7 +169,8 @@ public ContainerRequest(Resource capability, String[] nodes, * If true, containers for this request may be assigned on hosts * and racks other than the ones explicitly requested. * @param nodeLabelsExpression - * Set node labels to allocate resource + * Set node labels to allocate resource, now we only support + * asking for only a single node label */ public ContainerRequest(Resource capability, String[] nodes, String[] racks, Priority priority, boolean relaxLocality, diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java index e5e32e9..f8c1497 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java @@ -426,6 +426,8 @@ public synchronized void addContainerRequest(T req) { checkLocalityRelaxationConflict(req.getPriority(), dedupedRacks, true); checkLocalityRelaxationConflict(req.getPriority(), inferredRacks, req.getRelaxLocality()); + // check if the node label expression specified is valid + checkNodeLabelExpression(req); if (req.getNodes() != null) { HashSet dedupedNodes = new HashSet(req.getNodes()); @@ -591,6 +593,23 @@ private void checkLocalityRelaxationConflict(Priority priority, } } + /** + * Valid if a node label expression specified on container request is valid or + * not + * + * @param containerRequest + */ + private void checkNodeLabelExpression(T containerRequest) { + String exp = containerRequest.getNodeLabelExpression(); + + // Don't support specifying >= 2 node labels in a node label expression now + if (null != exp && exp.contains("&&")) { + throw new InvalidContainerRequestException( + "Cannot specify more than two node labels" + + " in a single node label expression"); + } + } + private void addResourceRequestToAsk(ResourceRequest remoteRequest) { // This code looks weird but is needed because of the following scenario. // A ResourceRequest is removed from the remoteRequestTable. A 0 container @@ -645,7 +664,9 @@ private void addResourceRequestToAsk(ResourceRequest remoteRequest) { resourceRequestInfo.containerRequests.add(req); } - resourceRequestInfo.remoteRequest.setNodeLabelExpression(labelExpression); + if (ResourceRequest.ANY.equals(resourceName)) { + resourceRequestInfo.remoteRequest.setNodeLabelExpression(labelExpression); + } // Note this down for next interaction with ResourceManager addResourceRequestToAsk(resourceRequestInfo.remoteRequest); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java index 35e6635..68b29b6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java @@ -18,8 +18,6 @@ package org.apache.hadoop.yarn.client.api.impl; -import com.google.common.base.Supplier; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -40,7 +38,6 @@ import java.util.Set; import java.util.TreeSet; -import org.junit.Assert; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.io.Text; import org.apache.hadoop.security.Credentials; @@ -75,6 +72,7 @@ import org.apache.hadoop.yarn.client.ClientRMProxy; import org.apache.hadoop.yarn.client.api.AMRMClient; import org.apache.hadoop.yarn.client.api.AMRMClient.ContainerRequest; +import org.apache.hadoop.yarn.client.api.InvalidContainerRequestException; import org.apache.hadoop.yarn.client.api.NMTokenCache; import org.apache.hadoop.yarn.client.api.YarnClient; import org.apache.hadoop.yarn.conf.YarnConfiguration; @@ -89,6 +87,7 @@ import org.apache.hadoop.yarn.util.Records; import org.junit.After; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -96,6 +95,8 @@ import org.mockito.stubbing.Answer; import org.mortbay.log.Log; +import com.google.common.base.Supplier; + public class TestAMRMClient { static Configuration conf = null; static MiniYARNCluster yarnCluster = null; @@ -148,7 +149,6 @@ public static void setup() throws Exception { racks = new String[]{ rack }; } - @SuppressWarnings("deprecation") @Before public void startApp() throws Exception { // submit new app @@ -675,21 +675,56 @@ public void testAskWithNodeLabels() { AMRMClientImpl client = new AMRMClientImpl(); - // add x, y to ANY + // add exp=x to ANY client.addContainerRequest(new ContainerRequest(Resource.newInstance(1024, - 1), null, null, Priority.UNDEFINED, true, "x && y")); + 1), null, null, Priority.UNDEFINED, true, "x")); Assert.assertEquals(1, client.ask.size()); - Assert.assertEquals("x && y", client.ask.iterator().next() + Assert.assertEquals("x", client.ask.iterator().next() .getNodeLabelExpression()); - // add x, y and a, b to ANY, only a, b should be kept + // add exp=x then add exp=a to ANY in same priority, only exp=a should kept client.addContainerRequest(new ContainerRequest(Resource.newInstance(1024, - 1), null, null, Priority.UNDEFINED, true, "x && y")); + 1), null, null, Priority.UNDEFINED, true, "x")); client.addContainerRequest(new ContainerRequest(Resource.newInstance(1024, - 1), null, null, Priority.UNDEFINED, true, "a && b")); + 1), null, null, Priority.UNDEFINED, true, "a")); Assert.assertEquals(1, client.ask.size()); - Assert.assertEquals("a && b", client.ask.iterator().next() + Assert.assertEquals("a", client.ask.iterator().next() .getNodeLabelExpression()); + + // add exp=x to ANY, rack and node, only resource request has ANY resource + // name will be assigned the label expression + // add exp=x then add exp=a to ANY in same priority, only exp=a should kept + client.addContainerRequest(new ContainerRequest(Resource.newInstance(1024, + 1), new String[] { "node1", "node2" }, null, Priority.UNDEFINED, true, + "y")); + Assert.assertEquals(4, client.ask.size()); + for (ResourceRequest req : client.ask) { + if (ResourceRequest.ANY.equals(req.getResourceName())) { + Assert.assertEquals("y", req.getNodeLabelExpression()); + } else { + Assert.assertNull(req.getNodeLabelExpression()); + } + } + } + + private void verifyAddRequestFailed(AMRMClient client, ContainerRequest request) { + try { + client.addContainerRequest(request); + } catch (InvalidContainerRequestException e) { + return; + } + Assert.fail(); + } + + @Test(timeout=30000) + public void testAskWithInvalidNodeLabels() { + AMRMClientImpl client = + new AMRMClientImpl(); + + // specified exp with more than one node labels + verifyAddRequestFailed(client, + new ContainerRequest(Resource.newInstance(1024, 1), null, null, + Priority.UNDEFINED, true, "x && y")); } private void testAllocation(final AMRMClientImpl amClient) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java index 35baa44..ac1cb47 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java @@ -487,10 +487,11 @@ public AllocateResponse allocate(AllocateRequest request) RMApp app = this.rmContext.getRMApps().get(applicationId); - // set label expression for Resource Requests + // set label expression for Resource Requests if resourceName=ANY ApplicationSubmissionContext asc = app.getApplicationSubmissionContext(); for (ResourceRequest req : ask) { - if (null == req.getNodeLabelExpression()) { + if (null == req.getNodeLabelExpression() + && ResourceRequest.ANY.equals(req.getResourceName())) { req.setNodeLabelExpression(asc.getNodeLabelExpression()); } } 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/SchedulerUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerUtils.java index 5d00009..c4900c3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerUtils.java @@ -231,11 +231,31 @@ public static void validateResourceRequest(ResourceRequest resReq, // if queue has default label expression, and RR doesn't have, use the // default label expression of queue - if (labelExp == null && queueInfo != null) { + if (labelExp == null && queueInfo != null + && ResourceRequest.ANY.equals(resReq.getResourceName())) { labelExp = queueInfo.getDefaultNodeLabelExpression(); resReq.setNodeLabelExpression(labelExp); } + // we don't allow specify label expression other than resourceName=ANY now + if (!ResourceRequest.ANY.equals(resReq.getResourceName()) + && labelExp != null && !labelExp.trim().isEmpty()) { + throw new InvalidResourceRequestException( + "Invailid resource request, queue=" + queueInfo.getQueueName() + + " specified node label expression in a " + + "resource request has resource name = " + + resReq.getResourceName()); + } + + // we don't allow specify label expression with more than one node labels now + if (labelExp != null && labelExp.contains("&&")) { + throw new InvalidResourceRequestException( + "Invailid resource request, queue=" + queueInfo.getQueueName() + + " specified more than one node label " + + "in a node label expression, node label expression = " + + labelExp); + } + if (labelExp != null && !labelExp.trim().isEmpty() && queueInfo != null) { if (!checkQueueLabelExpression(queueInfo.getAccessibleNodeLabels(), labelExp)) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/TestSchedulerUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/TestSchedulerUtils.java index c3ae38c..cc8e70f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/TestSchedulerUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/TestSchedulerUtils.java @@ -62,6 +62,8 @@ import org.apache.hadoop.yarn.exceptions.InvalidResourceBlacklistRequestException; import org.apache.hadoop.yarn.exceptions.InvalidResourceRequestException; import org.apache.hadoop.yarn.ipc.YarnRPC; +import org.apache.hadoop.yarn.nodelabels.CommonNodeLabelsManager; +import org.apache.hadoop.yarn.nodelabels.NodeLabelsStore; import org.apache.hadoop.yarn.server.resourcemanager.MockNM; import org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization.MockRMWithAMS; import org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization.MyContainerManager; @@ -213,11 +215,7 @@ public void testValidateResourceRequestWithErrorLabelsPermission() resReq.setNodeLabelExpression("x"); SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", scheduler); - - resReq.setNodeLabelExpression("x && y"); - SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", - scheduler); - + resReq.setNodeLabelExpression("y"); SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", scheduler); @@ -252,6 +250,8 @@ public void testValidateResourceRequestWithErrorLabelsPermission() } catch (InvalidResourceRequestException e) { } + // we don't allow specify more than two node labels in a single expression + // now try { // set queue accessible node labesl to [x, y] queueAccessibleNodeLabels.clear(); @@ -262,7 +262,7 @@ public void testValidateResourceRequestWithErrorLabelsPermission() YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_VCORES); ResourceRequest resReq = BuilderUtils.newResourceRequest( mock(Priority.class), ResourceRequest.ANY, resource, 1); - resReq.setNodeLabelExpression("x && y && z"); + resReq.setNodeLabelExpression("x && y"); SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", scheduler); fail("Should fail"); @@ -327,7 +327,7 @@ public void testValidateResourceRequestWithErrorLabelsPermission() SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", scheduler); - resReq.setNodeLabelExpression("x && y && z"); + resReq.setNodeLabelExpression("y"); SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", scheduler); @@ -336,7 +336,45 @@ public void testValidateResourceRequestWithErrorLabelsPermission() scheduler); } catch (InvalidResourceRequestException e) { e.printStackTrace(); - fail("Should be valid when request labels is empty"); + fail("Should be valid when queue can access any labels"); + } + + // we don't allow resource name other than ANY and specify label + try { + // set queue accessible node labesl to [x, y] + queueAccessibleNodeLabels.clear(); + queueAccessibleNodeLabels.addAll(Arrays.asList("x", "y")); + + Resource resource = Resources.createResource( + 0, + YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_VCORES); + ResourceRequest resReq = BuilderUtils.newResourceRequest( + mock(Priority.class), "rack", resource, 1); + resReq.setNodeLabelExpression("x"); + SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", + scheduler); + fail("Should fail"); + } catch (InvalidResourceRequestException e) { + } + + // we don't allow resource name other than ANY and specify label even if + // queue has accessible label = * + try { + // set queue accessible node labesl to * + queueAccessibleNodeLabels.clear(); + queueAccessibleNodeLabels.addAll(Arrays + .asList(CommonNodeLabelsManager.ANY)); + + Resource resource = Resources.createResource( + 0, + YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_VCORES); + ResourceRequest resReq = BuilderUtils.newResourceRequest( + mock(Priority.class), "rack", resource, 1); + resReq.setNodeLabelExpression("x"); + SchedulerUtils.validateResourceRequest(resReq, maxResource, "queue", + scheduler); + fail("Should fail"); + } catch (InvalidResourceRequestException e) { } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestContainerAllocation.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestContainerAllocation.java index b84717b..7ed0706 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestContainerAllocation.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestContainerAllocation.java @@ -563,12 +563,10 @@ public RMNodeLabelsManager createNodeLabelManager() { RMApp app1 = rm1.submitApp(1024, "app", "user", null, "a1"); MockAM am1 = MockRM.launchAndRegisterAM(app1, rm1, nm1); - // request a container (label = x && y). can only allocate on nm2 - am1.allocate("*", 1024, 1, new ArrayList(), "x && y"); + // request a container (label = y). can be allocated on nm2 + am1.allocate("*", 1024, 1, new ArrayList(), "y"); containerId = ContainerId.newInstance(am1.getApplicationAttemptId(), 2); - Assert.assertFalse(rm1.waitForState(nm1, containerId, - RMContainerState.ALLOCATED, 10 * 1000)); Assert.assertTrue(rm1.waitForState(nm2, containerId, RMContainerState.ALLOCATED, 10 * 1000)); checkTaskContainersHost(am1.getApplicationAttemptId(), containerId, rm1, @@ -604,12 +602,10 @@ public RMNodeLabelsManager createNodeLabelManager() { checkTaskContainersHost(am3.getApplicationAttemptId(), containerId, rm1, "h3"); - // try to allocate container (request label = y && z) on nm3 (label = y) and - // nm4 (label = y,z). Will sucessfully allocate on nm4 only. - am3.allocate("*", 1024, 1, new ArrayList(), "y && z"); + // try to allocate container (request label = z) on nm4 (label = y,z). + // Will successfully allocate on nm4 only. + am3.allocate("*", 1024, 1, new ArrayList(), "z"); containerId = ContainerId.newInstance(am3.getApplicationAttemptId(), 3); - Assert.assertFalse(rm1.waitForState(nm3, containerId, - RMContainerState.ALLOCATED, 10 * 1000)); Assert.assertTrue(rm1.waitForState(nm4, containerId, RMContainerState.ALLOCATED, 10 * 1000)); checkTaskContainersHost(am3.getApplicationAttemptId(), containerId, rm1,