hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jian He (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-2209) Replace AM resync/shutdown command with corresponding exceptions
Date Mon, 28 Jul 2014 22:35:38 GMT

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

Jian He commented on YARN-2209:
-------------------------------

Hi Zhijie, thanks for the review.  Here are some responses:
bq. Why is it necessary to use the exception instead of the flag to indicate the RM restarting?
I
Because as you can see, not just allocate API, unregisterResponse is also required to add
AMCommand otherwise. Basically, every AMS API other than register requires adding a new field
otherwise. Throwing exception is much cleaner way.
bq. For example, MR of prior versions will no longer work properly with a YARN cluster after
this patch during RM restarting.
Not matter how application is reacting to the shutdown command, NM will shoot down the AM
container during RM restart. So prior applications(including MR) should still work.  Even
earlier MR AM container is possibly killed by NM before it actually successfully performs
any shutting down logic.
bq. Deprecate the enum type instead of each enum value?
Maybe not deprecating AMCommand, as we may add other commands later on as needed.
bq. Why not throwing ApplicationAttemptNotFoundException instead? It sounds more reasonable
here, doesn’t it?
Do you mean creating a new ApplicationAttemptNotFoundException exception ? I think it's fine
to just reuse the ApplicationNotFoundException as they are quite similar. The internal exception
msg shows the attemptId.
bq. Is this change necessary?
It is. because the finally block (i.e. "if(allocateResponse == null)" ) will be executed otherwise.
bq. shall we split the patch into two pieces: one for YARN and the other for MR,
will split once review is done. I think it'll be easier to review with both side changes having
more context.
bq. No need to break it into two lines, right?
will fix it.

> Replace AM resync/shutdown command with corresponding exceptions
> ----------------------------------------------------------------
>
>                 Key: YARN-2209
>                 URL: https://issues.apache.org/jira/browse/YARN-2209
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Jian He
>            Assignee: Jian He
>         Attachments: YARN-2209.1.patch, YARN-2209.2.patch, YARN-2209.3.patch, YARN-2209.4.patch,
YARN-2209.5.patch
>
>
> YARN-1365 introduced an ApplicationMasterNotRegisteredException to indicate application
to re-register on RM restart. we should do the same for AMS#allocate call also.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message