[
https://issues.apache.org/jira/browse/YARN-6794?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16516505#comment-16516505
]
Miklos Szegedi commented on YARN-6794:
--------------------------------------
Thank you for the patch [~haibochen]. I have the following comments:
{code:java}
283 Resources.subtractFrom(allocatedResourceGuaranteed, resource);{code}
I think this should refer to allocatedResourceOpportunistic
{code:java}
526 Resource resource = rmContainer.getContainer().getResource();
527 if (Resources.fitsIn(resource, node.getUnallocatedResource())) {
528 node.promoteOpportunisticContainer(rmContainer);{code}
This is outside the critical section. The unallocated space may change after
the check but before the promotion.
{code:java}
529 attemptOpportunisticResourceUsage.decUsed(resource);
530 attemptResourceUsage.incUsed(resource);
531 getQueue().incUsedGuaranteedResource(resource);{code}
SchedulerApplicationAttempt.getResourceUsageReport may return a transient 0
since the write lock is not held at this point.
{code:java}
1391 public Resource getGuaranteedResourceUsage() {{code}
It might be safer to do a clone here and return a read only copy.
{code:java}
162 @Override
163 public void promoteOpportunisticContainer(RMContainer rmContainer) {
164 super.promoteOpportunisticContainer(rmContainer);
165 priorityContainers.remove(rmContainer);
166 }{code}
This might cause a double promotion. The function is not synchronized, so we do
the promotion but priorityContainers can be sampled again with the same
container listed before this finishes.
{code:java}
321 public List<RMContainer> getPriorityContainers() {{code}
It is probably a good idea to have this synchronized as well. The list can
change while you grab the snapshot.
{code:java}
1124 //boolean validReservation =
attemptToAssignReservedResources(node);{code}
?
{code:java}
1165 if (!assigned) {
1166 // break out of the loop because assigned being false
1167 // indicates there is no more resources are available.
1168 break;
1169 }{code}
Is this really true? I could imagine that we try but cannot assign a reserved
resource but we do not even try the opportunistic queue in that case.
> Fair Scheduler to explicitly promote OPPORTUNISITIC containers locally at the
> node where they're running
> --------------------------------------------------------------------------------------------------------
>
> Key: YARN-6794
> URL: https://issues.apache.org/jira/browse/YARN-6794
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, scheduler
> Reporter: Haibo Chen
> Assignee: Haibo Chen
> Priority: Major
> Attachments: YARN-6794-YARN-1011.00.patch,
> YARN-6794-YARN-1011.prelim.patch
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
