[
https://issues.apache.org/jira/browse/YARN-7419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16244601#comment-16244601
]
Wangda Tan commented on YARN-7419:
----------------------------------
Thanks [~suma.shivaprasad] for updating the patch, more comments:
1) CapacityScheduler:
1.1 Instead of fetching ApplicationPlacementContext from RMApp (to avoid
perf/locking issue and code flow becomes more clear), you can add the
ApplicationPlacementContext to {{AppAddedSchedulerEvent}}. And
getPlacementContext API can be removed from RMApp.
1.2 Move following if ... to addApplication:
{code}
if (placementContext != null) {
// ...
}
{code}
Like,
{code}
if (queue == null && placementContext != null) {
//Could be a potential auto-created leaf queue
}
{code}
Only enter the autoCreateLeafQueue function when necessary.
1.3 Following two catch exception can be merged by using {{YarnException |
IOException e}} syntax.
{code}
catch (YarnException e) {
LOG.error("Could not auto-create leaf queue due to : ", e);
final String message =
"Application " + applicationId + " submission by user " + user
+ " to queue: " + queueName + " failed : " + e.getMessage();
this.rmContext.getDispatcher().getEventHandler().handle(
new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
message));
} catch (IOException e) {
final String message =
"Application " + applicationId + " submission by user " + user
+ " to queue: " + queueName + " failed : " + e.getMessage();
LOG.error("Could not auto-create leaf queue due to : ", e);
this.rmContext.getDispatcher().getEventHandler().handle(
new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
message));
}
{code}
1.4 Following message is not clear enough:
{code}
String message =
"Application " + applicationId + " submission by user " + user
+ " to queue: " + queueName + " failed : "
+ "Queue mapping does not exist for user";
{code}
It should say directly specify a autocreated queue name is prohibited, it has
to be automatically mapped, etc.
1.5 I'm not sure if this check is necessary, I think previous logics should be
enough to detect this correct?
{code}
else if (!queue.getParent().getQueueName().equals(
placementContext.getParentQueue())) {
String message =
"Auto created Leaf queue " + placementContext.getQueue() + "
already exists under " + queue
.getParent().getQueuePath()
+ ".But Queue mapping has a different parent queue "
+ placementContext.getParentQueue()
+ " for the specified user : " + user;
this.rmContext.getDispatcher().getEventHandler().handle(
new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
message));
return;
}
{code}
1.6 clock is still here, move to a separate patch?
2) CapacitySchedulerConfiguration:
- getQueuePlacementRules is unused.
- Make sure all new added methods/fields are {{@Private}}
- {{FAIL_AUTO_CREATION_ON_EXCEEDING_CAPACITY}} is this necessary? Should we
just fail leaf queue creation when it exceeds parent queue's limit?
Renames:
- AutoCreatedLeafQueueTemplate.Builder#capacity => capacities
Unecesssary changes:
- CapacitySchedulerContext
- AbstractCSQueue
Miscs:
- Did you accidentally included YARN-6124 in this patch? Could you revert that
part?
> Implement Auto Queue Creation with modifications to queue mapping flow
> ----------------------------------------------------------------------
>
> Key: YARN-7419
> URL: https://issues.apache.org/jira/browse/YARN-7419
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacity scheduler
> Reporter: Suma Shivaprasad
> Assignee: Suma Shivaprasad
> Attachments: YARN-7419.1.patch, YARN-7419.2.patch, YARN-7419.3.patch,
> YARN-7419.patch
>
>
> This involves changes to queue mapping flow to pass along context information
> for auto queue creation. Auto creation of queues will be part of Capacity
> Scheduler flow while attempting to resolve queues during application
> submission. The leaf queues which do not exist are auto created under parent
> queues which have been explicitly enabled for auto queue creation . In order
> to determine which parent queue to create the leaf queues under - parent
> queues need to be specified in queue mapping configuration
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
