[ https://issues.apache.org/jira/browse/YARN-193?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13620066#comment-13620066 ]
Bikas Saha commented on YARN-193: --------------------------------- Can we check that we are getting the expected exception and not some other one? {code} + try { + rmService.submitApplication(submitRequest); + Assert.fail("Application submission should fail because"); + } catch (YarnRemoteException e) { + // Exception is expected + } + } {code} Setting the same config twice? In second set, why not use a -ve value instead of the DISABLE value? Its not clear whether we want to disable check or set a -ve value. same for others. {code} + conf.setInt(YarnConfiguration.RM_SCHEDULER_MINIMUM_ALLOCATION_MB, 0); + conf.setInt(YarnConfiguration.RM_SCHEDULER_MINIMUM_ALLOCATION_MB, + ResourceCalculator.DISABLE_RESOURCELIMIT_CHECK); + try { + resourceManager.init(conf); + fail("Exception is expected because the min memory allocation is" + + " non-positive."); + } catch (YarnException e) { + // Exception is expected. {code} Lets also add a test for case when memory is more than max. Normalize should always reduce that to max. Same for DRF {code} + // max is not a multiple of min + maxResource = Resources.createResource(maxMemory - 10, 0); + ask.setCapability(Resources.createResource(maxMemory - 100)); + // multiple of minMemory > maxMemory, then reduce to maxMemory + SchedulerUtils.normalizeRequest(ask, resourceCalculator, null, + minResource, maxResource); + assertEquals(maxResource.getMemory(), ask.getCapability().getMemory()); } {code} Rename testAppSubmitError() to show that its testing invalid resource request? TestAMRMClient. Why is this change needed? {code} + amResource.setMemory( + YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_MB); + amContainer.setResource(amResource); {code} Dont we need to throw? {code} + } catch (InvalidResourceRequestException e) { + LOG.info("Resource request was not able to be alloacated for" + + " application attempt " + appAttemptId + " because it" + + " failed to pass the validation. " + e.getMessage()); + RPCUtil.getRemoteException(e); + } {code} typo {code} + // validate scheduler vcors allocation setting {code} This will need to be rebased after YARN-382 which I am going to commit shortly. I am fine with requiring that a max allocation limit be set. We should also make sure that max allocation from config can be matched by at least 1 machine in the cluster. That should be a different jira. IMO, Normalization should be called only inside the scheduler. It is an artifact of the scheduler logic. Nothing in the RM requires resources to be normalized to a multiple of min. Only the scheduler needs it to makes its life easier and it could choose to not do so. > Scheduler.normalizeRequest does not account for allocation requests that > exceed maximumAllocation limits > --------------------------------------------------------------------------------------------------------- > > Key: YARN-193 > URL: https://issues.apache.org/jira/browse/YARN-193 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager > Affects Versions: 2.0.2-alpha, 3.0.0 > Reporter: Hitesh Shah > Assignee: Zhijie Shen > Attachments: MR-3796.1.patch, MR-3796.2.patch, MR-3796.3.patch, > MR-3796.wip.patch, YARN-193.10.patch, YARN-193.11.patch, YARN-193.4.patch, > YARN-193.5.patch, YARN-193.6.patch, YARN-193.7.patch, YARN-193.8.patch, > YARN-193.9.patch > > -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira