Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 17 Jul 2017 21:36:01 +0000 (UTC)
From: "Arun Suresh (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.13087646.1500312557000.267578.1500327361377@Atlassian.JIRA>
In-Reply-To: <JIRA.13087646.1500312557000@Atlassian.JIRA>
References: <JIRA.13087646.1500312557000@Atlassian.JIRA> <JIRA.13087646.1500312557679@jira-lw-us.apache.org>
Subject: [jira] [Commented] (YARN-6831) Miscellaneous refactoring changes of
 ContainScheduler
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
archived-at: Mon, 17 Jul 2017 21:36:09 -0000


    [ https://issues.apache.org/jira/browse/YARN-6831?page=3Dcom.atlassian.=
jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D16090=
609#comment-16090609 ]=20

Arun Suresh commented on YARN-6831:
-----------------------------------

Thanks for raising this [~haibochen] / [~kasha]. Some thoughts:

bq. Why do we need maxOppQueueLength given queuingLimit?
So, maxOppQueueLength is more like an *active* limit. The CS (ContainerSche=
duler) will not admit any more containers than that value. While the queuin=
gLimit is more *reactive* and dynamically calculated by the RM and passed d=
own to the NM in a HB response. The RM constantly calculates the mean/media=
n of the queueLengths on all nodes and it tells the NM to shed containers f=
rom the queue if it is too high. I agree that the *maxOppQueueLength* can p=
robably be removed though. But given your observation in YARN-6706 that tes=
t cases depends on this, my opinion is that we will keep it, and put a very=
 high value by default - and mark it as VisibileForTesting only.

bq. Is there value in splitting runningContainers into runningGuaranteed an=
d runningOpportunistic ?
Hmm=E2=80=A6 I was actually thinking of removing the *runningContainers* it=
self. It was introduced to keep track of all running containers (containers=
 whose state is running) AND those that have been scheduled but not yet run=
ning. I think it may be better to encapsulate that as a proper container st=
ate, something like *SCHEDULED_TO_RUN* via a proper transition.
Adding more data structures might be problematic later on, since we can hit=
 minor race conditions when transferring containers from runningGuaranteed =
to running Opportunistic (during promotion) and vice-versa (during demotion=
) if we are not careful about synchronization etc. Also, given the fact tha=
t a NM will not run more than say a couple of 100 containers, it might be b=
etter to just iterate over all the containers when the scheduler needs to m=
ake a decision.
Another problem with keeping a separate map is during NM recovery, we have =
to populate this specifically. we don=E2=80=99t do that for running contain=
ers now either =E2=80=93 but I was think if we removed the *runningContaine=
rs* map, we wont have to (we already have a state called *QUEUED* in the NM=
StateStore which can be used to set the correct state in the recovered cont=
ainer)

bq. getOpportunisticContainersStatus method implementation feels awkward..
Kind of agree with you there, don=E2=80=99t recall exactly why we did it li=
ke that=E2=80=A6 think it was to not have to create a new instance of the s=
tatus at every heart beat.=20

bq. Have we considered folding ContainerQueuingLimit class into this
My first instinct is to keep it separate. Don=E2=80=99t think we should mix=
 the Queuing aspect of the Container Scheduler with the ExecutionType aspec=
t. Also, one is part of the NM heartbeat request and the other comes back a=
s response.


> Miscellaneous refactoring changes of ContainScheduler=20
> ------------------------------------------------------
>
>                 Key: YARN-6831
>                 URL: https://issues.apache.org/jira/browse/YARN-6831
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager
>            Reporter: Haibo Chen
>            Assignee: Haibo Chen
>
> While reviewing YARN-6706, Karthik pointed out a few issues for improvmen=
t in ContainerScheduler
> *Make ResourceUtilizationTracker pluggable. That way, we could use a diff=
erent tracker when oversubscription is enabled.
> *ContainerScheduler
>   ##Why do we need maxOppQueueLength given queuingLimit?
>   ##Is there value in splitting runningContainers into runningGuaranteed =
and runningOpportunistic?
>   ##getOpportunisticContainersStatus method implementation feels awkward.=
 How about capturing the state in the field here, and have metrics etc. pul=
l from here?
>   ##startContainersFromQueue: Local variable resourcesAvailable is unnece=
ssary
> *OpportunisticContainersStatus
>   ##Let us clearly differentiate between allocated, used and utilized. Ma=
ybe, we should rename current Used methods to Allocated?
>   ##I prefer either full name Opportunistic (in method) or Opp (shortest =
name that makes sense). Opport is neither short nor fully descriptive.
>   ##Have we considered folding ContainerQueuingLimit class into this?
> We decided to move the issues into this follow up jira to keep YARN-6706 =
moving forward to unblock oversubscription work.


--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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