hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Wangda Tan (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-6216) Unify Container Resizing code paths with Container Updates making it scheduler agnostic
Date Thu, 23 Feb 2017 19:20:44 GMT

    [ https://issues.apache.org/jira/browse/YARN-6216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15881061#comment-15881061
] 

Wangda Tan commented on YARN-6216:
----------------------------------

Hi [~asuresh], the patch looks GREAT and simplified a lot of logics which is added for container
resizing, thanks for working on this!

*Major:*

1) It looks like all container change request needs to create a new containerId, correct?
It's not a critical issue, but could potentially make the container-id becomes very sparse.
I think this should be optimizable.

2) Could you elaborate new changes added to AbstractYarnScheduler#completedContainer?

3) Can we simplify decrease/demote container process: now it is create a new RMContainer and
make changes to existing container when pulled by app, could we change it in-place which can
benefit: A. no need to create a new RMContainer, B. no need to add them to "outstanding decrease/demote
requests", C. pull new demoted/decreased containers logics can be simplified in SchedulerApplicationAttempt.

4) Not critical but worth to think: for increase/promote container, it increase #live-containers
by 1 in scheduler and soon decrease it. Since #live-containers is visible to client, if changes
to fix the issue is not too complex, it's better to optimize this.

*Minor:*

5) SchedulerApplicationAttempt:
- IIUC, createResourceToShed is to assigned to the new RMContainer placeholder to make scheduler
can correctly release resources. Could you add some comments and rename it to "getResourceOfRMContainerPlaceholder"
- A couple of rename suggestions to {{swapContainer}}: {{updatedContainer}} to {{tempContainer}};
{{finalResource}} to {{updatedResource}}; {{newContainer}} to {{updatedContainer}}; {{rmContainer}}
to {{tempRmContainer}}
- Typo: updatType -> updateType

6) ContainerAllocator: 
Not necessary to call regularContainerAllocator.assignContainers twice, once should be enough.
And not necessary to check reservedContainer anymore.

7) Not caused by your patch, testOrderOfIncreaseContainerRequestAllocation and testIncreaseContainerRequestGetPreferrence
are completely identical, you can remove one of them. 

8) Several unused methods in AppSchedulingInfo:
- hasRequestLabelChanged/getFirstSchedulingPlacementSet



> Unify Container Resizing code paths with Container Updates making it scheduler agnostic
> ---------------------------------------------------------------------------------------
>
>                 Key: YARN-6216
>                 URL: https://issues.apache.org/jira/browse/YARN-6216
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: capacity scheduler, fairscheduler, resourcemanager
>    Affects Versions: 3.0.0-alpha2
>            Reporter: Arun Suresh
>            Assignee: Arun Suresh
>             Fix For: 3.0.0-alpha3
>
>         Attachments: YARN-6216.001.patch, YARN-6216.002.patch
>
>
> YARN-5959 introduced an {{ContainerUpdateContext}} which can be used to update the ExecutionType
of a container in a scheduler agnostic manner. As mentioned in that JIRA, extending that to
encompass Container resizing is trivial.
> This JIRA proposes to remove all the CapacityScheduler specific code paths. (CapacityScheduler,
CSQueue, FicaSchedulerApp etc.) and modify the code to use the framework introduced in YARN-5959
without any loss of functionality.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message