[
https://issues.apache.org/jira/browse/YARN-1493?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850929#comment-13850929
]
Sandy Ryza commented on YARN-1493:
----------------------------------
Thanks Jian. A question and some stylistic issues. Other than that, after
SchedulerApplicationWrapper is replaced with SchedulerApplication, the patch
looks good to me.
Will the scheduler ever be in a state where an application has been removed but
an attempt from that app still remains? My understanding is no, but I wanted
to make sure.
{code}
+public class SchedulerApplicationWrapper {
+
+ Queue queue;
+ String user;
+
{code}
These member variables should be private and final (or labeled
@VisibleForTesting if that's why they're not).
{code}
+ this.rmContext.getDispatcher().getEventHandler()
+ .handle(new RMAppRejectedEvent(applicationId, message));
{code}
Second line should be indented four spaces. This applies in a number of other
places as well.
{code}
+ protected Map<ApplicationId, SchedulerApplicationWrapper> applications = new
+ ConcurrentHashMap<ApplicationId, SchedulerApplicationWrapper>();
{code}
"new" should be on the second line
{code}
- /**
- * Add a new application to the scheduler, with a given id, queue name, and
- * user. This will accept a new app even if the user or queue is above
- * configured limits, but the app will not be marked as runnable.
- */
{code}
No reason to remove these comments for FairScheduler.addApplication.
addApplicationAttempt should have updated comments.
{code}
+ LOG.info("Succeeded to submit application " + applicationId + " to queue "
+ + queueName + " from user " + user);
{code}
Should be "Accepting application applicationId from user userName in queue
queueName"
> Schedulers don't recognize apps separately from app-attempts
> ------------------------------------------------------------
>
> Key: YARN-1493
> URL: https://issues.apache.org/jira/browse/YARN-1493
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Jian He
> Assignee: Jian He
> Attachments: YARN-1493.1.patch, YARN-1493.2.patch, YARN-1493.3.patch
>
>
> Today, scheduler is tied to attempt only.
> We need to separate app-level handling logic in scheduler. We can add new
> app-level events to the scheduler and separate the app-level logic out. This
> is good for work-preserving AM restart, RM restart, and also needed for
> differentiating app-level metrics and attempt-level metrics.
--
This message was sent by Atlassian JIRA
(v6.1.4#6159)
