[
https://issues.apache.org/jira/browse/YARN-1241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13790846#comment-13790846
]
Karthik Kambatla commented on YARN-1241:
----------------------------------------
Good to see we are separating out runnable and non-runnable apps - keen to see
how much this improves the schedule-time overhead. Comments below:
{code}
+ private final List<AppSchedulable> appScheds = // apps that are runnable
{code}
How about renaming this field to runnableAppScheds?
{code}
+ public void makeAppRunnable(AppSchedulable appSched) {
{code}
Should we make this a synchronized method along with other accessors of
runnable and non-runnable appScheds? In case, we make the scheduler
multi-threaded?
{code}
if (Resources.equals(demand, maxRes)) {
break;
}
{code}
Making a mental note. The assumption here is that we don't use demand to
determine fairness between schedulables. Correct?
{code}
for (AppSchedulable sched : nonRunnableAppScheds) {
if (Resources.equals(demand, maxRes)) {
break;
}
updateDemandForApp(sched, maxRes);
}
{code}
Should we even consider the demand for non-runnable schedulables? Would it make
sense to ignore the non-runnable schedulables for all book-keeping until they
become runnable?
In FairScheduler, the PriorityQueue import looks spurious.
MaxRunningAppsEnforcer should be singleton?
{code}
private ListMultimap<String, AppSchedulable> usersNonRunnableApps;
{code}
I am assuming the ListMultimap choice is to maintain the ordering and give
preference to the earliest submitted job within a queue. Wondering if it would
make sense to use any other metric than submit time for "improved fairness"?
{code}
* Runs in O(n log(n)) where n is the number of queues under that are under
* the highest queue that went from having no slack to having slack.
{code}
The comment should have a single under?
{code}
// Update runnable app bookkeeping for queues
// Find the queue highest in the tree which the app removal means something
// new can be run
FSQueue highestQueueAppsNowRunnable =
(queue.getNumRunnableApps() ==
queueMgr.getQueueMaxApps(queue.getName()) - 1)
? queue : null;
FSParentQueue parent = queue.getParent();
while (parent != null) {
if (parent.getNumRunnableApps()
== queueMgr.getQueueMaxApps(parent.getName())) {
highestQueueAppsNowRunnable = parent;
}
parent.decrementRunnableApps();
parent = parent.getParent();
}
{code}
The comment is a little confusing. Rename highestQueueAppsNowRunnable to
highestQueueWithAppsNowRunnable? Also, I am a little confused here. IIUC, we
are trying to check if the completing of this app makes any other applications
runnable now? Shouldn't we break out of the loop as soon as we encounter a
queue for with zero non-runnable apps? Also, would be nice to have separate
unit tests for MaxRunningAppsEnforcer.
> In Fair Scheduler maxRunningApps does not work for non-leaf queues
> ------------------------------------------------------------------
>
> Key: YARN-1241
> URL: https://issues.apache.org/jira/browse/YARN-1241
> Project: Hadoop YARN
> Issue Type: Bug
> Affects Versions: 2.1.0-beta
> Reporter: Sandy Ryza
> Assignee: Sandy Ryza
> Attachments: YARN-1241-1.patch, YARN-1241-2.patch, YARN-1241-3.patch,
> YARN-1241.patch
>
>
> Setting the maxRunningApps property on a parent queue should make it that the
> sum of apps in all subqueues can't exceed it
--
This message was sent by Atlassian JIRA
(v6.1#6144)